Clean up message_ix.macro

[behnam-zakeri]

This PR improves the code in message_ix.macro.py for the following items. Some of these issues were identified when #223 was conditionally merged, and some items are additional improvements.

• testing zero or missing values in PRICE_COMMODITY before calibration
• removing hard-coded units in the code, and specifying them by user in the input data per requested in Address hardcoded units MACRO calibration and M-M linkage #317
• improving configuration for sets and specifying the "level" for macro outside the code (partly addressing Address hardcoded units MACRO calibration and M-M linkage #317)
• mapping MACRO sectors to MESSAGE commodities (and level) explicitly by the user
• removing old initialize_year_macro before calibration (partly addressing Clarify add_macro() behaviour on a non-'clean' Scenario #320)
• removing baseyear_macro from the code (not needed in GAMS anymore)
• specifying MACRO years by user in the Excel "config" sheet, and slicing and reading data flexibly based on that.
• adding tests and modifying the input data files to reflect the above changes
• adding docstring (addressing message_ix.macro lacks minimal documentation #315)

How to review

New tests are added to check the improvements in this PR. You can view changes in the code and run the code and tests under test_macro.py.
Notice: if you wish to test the code on a input data file, please update your data for:

• "config": in the new format, there are three columns ("node, "sector", "level") in "config" to specify those members of these sets that are included in MACRO calibration. Please see the sample input file in: message_ix/tests/data/westeros_macro_input.xlsx

• you need to add a "unit" column to all input parameters for specifying the unit (Please see the format in the file above).

• Tests added.

• These improvements shouldn't change the main behaviour and documentation. However, this will change the format of the input data file. This has been explained in the documentation.

• Release notes added, including the "migration notes" for the format of the input data file.

[codecov]

Codecov Report

Merging #327 (8fc49b8) into main (5526f43) will increase coverage by 0.0%.
The diff coverage is n/a.

@@          Coverage Diff          @@
##            main    #327   +/-   ##
=====================================
  Coverage   92.6%   92.7%           
=====================================
  Files         41      41           
  Lines       3167    3236   +69     
=====================================
+ Hits        2934    3000   +66     
- Misses       233     236    +3     

Impacted Files	Coverage Δ
message_ix/message_ix/tests/test_macro.py	99.4% <0.0%> (-0.6%)	⬇️
message_ix/message_ix/macro.py	96.4% <0.0%> (-0.5%)	⬇️
message_ix/message_ix/core.py	95.2% <0.0%> (ø)

[volker-krey]

I have some smaller suggestions to revise the macro_input.xlsx file.

1. I would suggest to add year columns to price_ref, demand_ref and cost_ref parameters so that there is no ambiguity about which year the values provided refer to.
2. In my view, it would be beneficial to explicitly define the years that MACRO is calibrated for. On the one hand, this would avoid mistakes with unintentionally added years in parameters like gdp_calibrate from which the years would need to be inferred otherwise. In addition, it allows flexibility to have more data in a file that is used (e.g., one could use the same file parameterizing a 10-year and a 5-year time step model version by just changing the year entries on the config tab).
3. The sector and commodity sets used by MACRO and MESSAGE don't have to use the same entries (although at present this is the case in all applications). I would therefore suggest to have an explicit mapping of (MACRO) sectors to (MESSAGE) commodity.level combinations (see example).

I have implemented the suggested modifications in the attached multiregion_macro_input.xlsx file with the changes marked in yellow.
multiregion_macro_input.xlsx

[behnam-zakeri]

Thanks @volker-krey for the suggestions and the sample file. I'll include them. I'll also try to see if we can make the calibration run for different lastmodel_years in the same model. That should also make it easier to calibrate a scenario with different last model year using the same input data file.

[GamzeUnlu95]

I would suggest adding another line here to be able to obtain the unique mapping of the different sector, commodity, and level combinations.

self.sector_mapping = self.data["config"]

[behnam-zakeri]

Thanks @GamzeUnlu95 for the suggestion. Can you please make a pull request and suggest your changes?

[GamzeUnlu95]

Done in this PR.

[GamzeUnlu95]

Here we assume sector and commodity names are the same. We could differentiate this by using a mapping table obtained from the excel "config" sheet including a commodity column.

    sector_mapping_df = c.sector_mapping

    for i in sector_mapping_df.index:
        s = sector_mapping_df.iloc[i, sector_mapping_df.columns.get_loc("sector")]
        c = sector_mapping_df.iloc[i, sector_mapping_df.columns.get_loc("commodity")]
        l = sector_mapping_df.iloc[i, sector_mapping_df.columns.get_loc("level")]
        clone.add_set('sector', s)
        clone.add_set("mapping_macro_sector", [s, c, l])

[behnam-zakeri]

@GamzeUnlu95 , nice suggestion. If possible, please make a PR and add these suggestions in the code, and in the Excel file, so that I can run and approve them.

[GamzeUnlu95]

Done in this PR.

[khaeru]

Okay, I have hopefully fixed the unit errors mentioned in #327 (review) and #327 (review). I also applied latest black and rebased on main. Please git pull --rebase to retrieve the latest code.

[khaeru]

Some comments on the two remaining checkboxes above:

• These improvements shouldn't change the main behaviour and documentation.

Per "documentation", since there wasn't any (#315), no risk. I added a placeholder section in doc/macro.rst; I would strongly suggest we copy the description of the input file format from this PR description into some clear, simple sentences there. Without this (i.e. saying only "look at the test data specimen"), the input file format is completely undocumented and users have little hope to independently construct one.

• Release notes will be updated after the final approval.

I added the PR to the release notes. However, it's important to consider: will existing users need to change anything if they start using this code, i.e. compared to what was released in v3.4, in order to avoid unexpected results? This must be added in a "Migration notes" subsection above "All changes".

[behnam-zakeri]

Thanks @khaeru for resolving the technical issues and improving the code. I added documentation for the input data file, and also a few sentences as "Migration Notes" to warn that these changes will not work with the previous versions of the Excel data file.

[khaeru]

Excellent. That looks like exactly what we need, so I will request a review and then @LauWien can dispatch that to an available colleague, e.g. as you suggested in Slack.

[LauWien]

@LauWien can dispatch that to an available colleague, e.g. as you suggested in Slack.

How soon should the PR get reviewed (E.g. best by the end of this week or end of next week)? Many thanks in advance! This helps to find a colleague :)

[khaeru]

End of next week is probably fine.

[LauWien]

Looks great :)
Just some suggestions.

[khaeru]

@LauWien thanks for the review—please merge once checks again pass.