Add bilateralization tool (project/newpathways-trade)

[junukitashepard]

Overview

The merged code allows users to represent trade bilaterally for specified commodities. By default, the tool bilateralizes existing fuel trade (e.g., oil_exp).

The bilateralization tool, bilateralize, is generalized for any traded commodity, whether that is a fuel (e.g., LNG), or a material (e.g., steel). It also explicitly represents bilateral trade “flows”, or how a fuel/commodity is transported from exporter to importer. These flow technologies are user defined and flexible; the most common are pipelines (e.g., gas pipelines), maritime shipping (e.g., LNG tanker), and transmission lines.

See https://github.com/iiasa/message-ix-models/blob/project/newpathways-trade/doc/api/tools-bilateralize.rst for more details on implementation.

How to review

Note: All scripts are additions (no existing scripts are amended). Therefore, I would request reviewing code directly rather than individual commits (of which there are >200).

These review steps are also outlined in https://github.com/iiasa/message-ix-models/blob/project/newpathways-trade/message_ix_models/tools/bilateralize/workflow.py

1. The function prepare_edit_files() should produce untracked files in commodity-specific directories in data/bilateralize/edit_files and also transfer required parameter files to data/bilateralize/bare_files
2. The function bare_to_scenario() should produce a dictionary of required parameter updates for bilateralization for all specified trade commodities in config.yaml.
3. The function load_and_solve() should be able to be used to update SSP base scenarios
4. The documentation should make sense even though it is incomplete. It is currently conceptual and still needs function documentation.

PR checklist

• Continuous integration checks all ✅
• Add or expand tests; coverage checks both ✅
• Add, expand, or update documentation.
• Update doc/whatsnew.

[codecov]

Codecov Report

❌ Patch coverage is 53.55619% with 653 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.7%. Comparing base (cb47229) to head (fba380f).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...odels/tools/bilateralize/historical_calibration.py	28.8%	212 Missing ⚠️
...age_ix_models/tools/bilateralize/load_and_solve.py	11.3%	140 Missing ⚠️
message_ix_models/tools/bilateralize/pull_gem.py	18.0%	127 Missing ⚠️
...e_ix_models/tools/bilateralize/bare_to_scenario.py	57.0%	61 Missing ⚠️
..._models/tools/bilateralize/mariteam_calibration.py	10.1%	53 Missing ⚠️
...ssage_ix_models/tools/bilateralize/prepare_edit.py	88.7%	47 Missing ⚠️
message_ix_models/tools/bilateralize/workflow.py	0.0%	8 Missing ⚠️
message_ix_models/tools/bilateralize/utils.py	90.3%	3 Missing ⚠️
...ix_models/tools/bilateralize/calculate_distance.py	95.5%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main    #438     +/-   ##
=======================================
- Coverage   75.6%   73.7%   -2.0%     
=======================================
  Files        278     290     +12     
  Lines      22740   24146   +1406     
=======================================
+ Hits       17201   17805    +604     
- Misses      5539    6341    +802     

Files with missing lines	Coverage Δ
.../tools/bilateralize/test_historical_calibration.py	100.0% <100.0%> (ø)
...x_models/tests/tools/bilateralize/test_pull_gem.py	100.0% <100.0%> (ø)
message_ix_models/tests/tools/test_bilateralize.py	100.0% <100.0%> (ø)
...ix_models/tools/bilateralize/calculate_distance.py	95.5% <95.5%> (ø)
message_ix_models/tools/bilateralize/utils.py	90.3% <90.3%> (ø)
message_ix_models/tools/bilateralize/workflow.py	0.0% <0.0%> (ø)
...ssage_ix_models/tools/bilateralize/prepare_edit.py	88.7% <88.7%> (ø)
..._models/tools/bilateralize/mariteam_calibration.py	10.1% <10.1%> (ø)
...e_ix_models/tools/bilateralize/bare_to_scenario.py	57.0% <57.0%> (ø)
message_ix_models/tools/bilateralize/pull_gem.py	18.0% <18.0%> (ø)
... and 2 more

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[khaeru]

Thanks for the work here! In order to merge the following are needed, and addressing these things first will help reviewers focus more squarely on the added contents and functionality.

• The "PR checklist" in the description should be all complete ✅ or struck out with comments; see here. Per the individual items:
  • "Continuous integration checks all ✅" —I see that there are a few, unrelated tests ERRORing on the "pytest" workflow jobs, but I am also seeing the same errors at Transport improvements for 2025-W39 #430. So I'll address those errors in that PR or another one, and then by rebasing this branch.
  • "Add or expand tests; coverage checks both ✅" —I think this is artificially low because the tests functions are incorrectly decorated and thus do not run. See my comment inline.
  • "Add, expand, or update documentation" —this seems to be done, right? If so, the box can be checked. If not (e.g. there is a plan to still add more documentation in the current PR), please describe.
  • "Update doc/whatsnew" —this could be 1 commit adding 1 line to the file that links to the already-written documentation page. See the blame for the file for examples of prior additions, e.g. ones that added new .tools submodules.
• We prefer a linear history. This can be achieved with git rebase main, which will take all the commits on this branch and rebuild the branch starting from the latest commit on main.
• Note the bullet on commit messages in the "Code style" docs. To change earlier commit messages to meet the style, there are a few approaches, mainly involving git rebase -i:
  • Use reword and edit the commit message.
  • Reorder and squash commits together. When doing this, the commit message (including subject line) for the second, third, etc. commits can be discarded or edited into the first. This is also a strategy for getting rid of commits that might make the history confusing to read.

Many members of the team have experience doing each of these steps and can help by checking work, pair programming, or joining in to do the changes directly, so please reach out on Slack or otherwise.

[yiyi1991]

Hi Jun, here are some points from mine.

• Do we already or can we open an issue for the bilateralization? Tracking how at the end it may look like, to-do list, what is covered by which PR, what is going to be covered by which PR, etc. You then do not have to tackle all the points listed below in this PR. Feel free to take some to the issue list and process later.

• The design of the template config is comprehensive! I like it a lot.

• [Term clarification] The following terms need to be better defined in the documentation: what it is, it is an accounting technology or a real technology with a lifetime, general examples, and the corresponding technology name in the latest R12 scenario.

  • trade technology
  • flow technology
  • and two other categories, supply technology and bunker technology (the naming of these two is not as self-explanatory as trade tech and flow tech, but fare enough)

• [De-project-ization] In general we should avoid describing a work by project names. I also realized that every time you call project_name, you do not mean a specific combination of bilateralized commodities (A+B+C commodities for project X, D commodity for project Y), but simply the path that users are working on. If my understanding is correct, can we just use path_name instead?

• [Workflow] I myself prefer a separate build, solve, and report. Binding build and solve together and providing another option to save gdx also makes sense. But the name of the function load_and_solve() is too confusing. Will you consider build, or at least build_and_solve? Another confusing one is bare_to_scenario(). In the definition of the functions, it will also be useful to clarify what will be returned. Key users want to know which step a Scenario object is going to be taken in.

prepare_edits() -> returns editable csv files
bare_to_scenario() -> returns a dictionary (does not return a scenario!)
load_and_solve() -> returns a (solved) scenario

• [Workflow refactor] Are you familiar with the current make_spec() (require/add/remove), apply_spec(), and add_par_data() workflow used in build materials/water/transportation/cost tool? You may find that some of your add_trade_data() can apply add_par_data(). I do not insist on a refactor. As I was able to build bilateralization of all commodities covered by the current default config on a recent R12 scenario in 14 minutes on my machine. Nothing to complain about the performance.

• [Generate commodity/technology list] It will be great if new trade and flow technologies, as well as new trade commodities, are generated in the commodity.yaml or technology.yaml too. Maybe you are already working on this in the report PR. This autogeneration is also one benefit of make_spec() -> apply_spec() -> add_par_data() style.

• [Tutorial] One showcase of how users can use it (e.g., higher cost of a specific trade pair on a specific shipped commodity) so they understand what they do NOT need to touch in a default setup. But the whole tutorial part can be done later.

After code quality reaches the target test coverage and the UnicodeDecodeError on Windows is tackled properly, I would happily approve this merge.

[yiyi1991]

Regarding the UnicodeDecodeError that is causing failures in tests on all Windows environments, I got rid of it simply by using with open(config_path, "r", encoding="utf-8") as f: here and the other 5 places. Maybe there are more proper ways to tackle this.

[yiyi1991]

I was able to build bilateralization of all commodities covered by the current default config on a recent R12 scenario with the first model year as 2020. So the historical calibrations also work.

Jun, please kindly let me know when you plan to expand the default list to electricity trade. I am interested in properly using your bilateralization tool, finishing the historical activity calibration of it, removing the current electricity trade technologies, and introducing the default assumption of future intercontinental UHV grids (...if I am able to squeeze some time in the upcoming spring).

[yiyi1991]

It seems even if a user specified a config bilateralizing only one non-energy commodity trade (e.g., sponge_iron) without bilateralizing other energy commodities, this coal_constraint will be removed anyways. Is it designed in this way?

[khaeru]

OS-specific files like this should not be committed. To hide them from Git, you could add them to the top-level .gitignore file, e.g. here is a file that I believe is macOS-specific:

message-ix-models/.gitignore

Line 149 in 6bf4000

.DS_Store

[khaeru]

Overall it's great to see documentation in many places here. Please carefully read and apply the docstring style linked from our code style docs. Some points include:

• Short summary is a single line, ends with a period.
• Wrap at 88 characters.
• Use the numpydoc format for parameter lists.
• Use reST and check the formatting. For instance, in this particular docstring, the numbered list 1. … is not preceded by a blank line, so will not be parsed as a list.

[khaeru]

Please see the documentation code style per "semantic line breaks".

As an intermediate step, start at least every sentence on a new line.

[khaeru]

Please see the existing documentation and the Sphinx docs (1, 2, 3) for how to use 'roles'.

Some that could be used in this file include:

:file:`message_ix_models/data/bilateralize/configs/base_config.yaml`
:func:`.bilateralize.prepare_edit.generate_edit_files`
:mod:`.bilateralize.workflow`

[khaeru]

I suspect these 3 broadcast_*() functions (~150 lines) could be replaced with use of the existing utility function. Is there some specific behaviour needed here that the existing code does not provide?

[junukitashepard]

Indeed, I was able to cut down significantly using the broadcast function.
Commit b925b45