Skip to content

Conversation

@daymontas1
Copy link
Contributor

@daymontas1 daymontas1 commented Mar 25, 2024

This PR introduces the option to solve for all regions concurrently in MACRO. A parameter (''solve_param'') has been added to ''MESSAGE-MACRO'' to control the solving mechanism for MACRO. Setting ''solve_param'' to 1 (the default value) triggers sequential solving across regions, whereas setting it to 2 enables concurrent solving.

An if statement has also been introduced in ''macro_solve'' that dictates whether sequential or concurrent solving is applied in MACRO, based on the value of the ''solve_param'' parameter defined in ''MESSAGE-MACRO_run''. The default solving is sequential (''solve_param''=1), so if the user doesn't update the solving mechanism, MACRO is solved as it used to be. In terms of efficiency, the concurrent solving is subtly more efficient.

For the concurrent solving mechanism (''solve_param''=2), all nodes are activated from the beginning, thereby enabling simultaneous solving for all regions. This setting allows for factoring in interregional interactions into the optimization process. We want this concurrent optimization option, as we intend to incorporate interregional feedback mechanisms into MACRO, e.g., interregional investments. Given that in the current version of MACRO regions are independent, both solving mechanisms should produce the same results.

How to review

Required: It must be verified that both versions produce the same results (considering there are no interregional interactions in the current version of MACRO).

If the changes are accepted, the model's documentation must be updated to include the concurrent option and instructions on how to trigger it. Additionally, it may be worthwhile to consider making the concurrent solving mechanism for MACRO the default, in case it reduces solving times.

PR checklist

  • Continuous integration checks all ✅
  • Add or expand tests; coverage checks both ✅
  • Add, expand, or update documentation.
  • Update release notes.

@CLAassistant
Copy link

CLAassistant commented Mar 25, 2024

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Mar 25, 2024

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.0%. Comparing base (6ee45ca) to head (842067e).
⚠️ Report is 99 commits behind head on main.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #808   +/-   ##
=====================================
  Coverage   96.0%   96.0%           
=====================================
  Files         53      53           
  Lines       4988    4998   +10     
=====================================
+ Hits        4791    4801   +10     
  Misses       197     197           
Files with missing lines Coverage Δ
message_ix/models.py 99.1% <100.0%> (+<0.1%) ⬆️
message_ix/tests/test_macro.py 100.0% <100.0%> (ø)

@glatterf42
Copy link
Member

Thanks for this PR @daymontas1 :)

Currently, all test failures complain about a GAMS compilation error:

FAILED message_ix/tests/test_macro.py::test_calc_valid_years - ixmp.model.base.ModelError: GAMS errored with return code 2:
    There was a compilation error

For details, see the terminal output above, plus:
Listing   : /Users/runner/work/message_ix/message_ix/message_ix/model/MACRO_run.lst
Log file  : /Users/runner/work/message_ix/message_ix/message_ix/model/MACRO_run.log
Input data: /Users/runner/work/message_ix/message_ix/message_ix/model/data/MsgData_Westeros_Electrified_test_calc_valid_years_macro.gdx
FAILED message_ix/tests/test_macro.py::test_calc_data_missing_ref - ixmp.model.base.ModelError: GAMS errored with return code 2:
    There was a compilation error

For details, see the terminal output above, plus:
Listing   : /Users/runner/work/message_ix/message_ix/message_ix/model/MACRO_run.lst
Log file  : /Users/runner/work/message_ix/message_ix/message_ix/model/MACRO_run.log
Input data: /Users/runner/work/message_ix/message_ix/message_ix/model/data/MsgData_Westeros_Electrified_test_calc_data_missing_ref_macro.gdx
FAILED message_ix/tests/test_macro.py::test_calibrate - ixmp.model.base.ModelError: GAMS errored with return code 2:
    There was a compilation error

For details, see the terminal output above, plus:
Listing   : /Users/runner/work/message_ix/message_ix/message_ix/model/MACRO_run.lst
Log file  : /Users/runner/work/message_ix/message_ix/message_ix/model/MACRO_run.log
Input data: /Users/runner/work/message_ix/message_ix/message_ix/model/data/MsgData_Westeros_Electrified_test_macro_calibration.gdx
FAILED message_ix/tests/test_macro.py::test_calibrate_roundtrip - ixmp.model.base.ModelError: GAMS errored with return code 2:
    There was a compilation error

For details, see the terminal output above, plus:
Listing   : /Users/runner/work/message_ix/message_ix/message_ix/model/MACRO_run.lst
Log file  : /Users/runner/work/message_ix/message_ix/message_ix/model/MACRO_run.log
Input data: /Users/runner/work/message_ix/message_ix/message_ix/model/data/MsgData_Westeros_Electrified_test_calibrate_roundtrip_macro.gdx
FAILED message_ix/tests/test_macro.py::test_sector_map - ixmp.model.base.ModelError: GAMS errored with return code 2:
    There was a compilation error

For details, see the terminal output above, plus:
Listing   : /Users/runner/work/message_ix/message_ix/message_ix/model/MACRO_run.lst
Log file  : /Users/runner/work/message_ix/message_ix/message_ix/model/MACRO_run.log
Input data: /Users/runner/work/message_ix/message_ix/message_ix/model/data/MsgData_Westeros_Electrified_test_sector_map_macro.gdx

If you run these tests on your system, do they work? If not, could you please try troubleshooting what's blocking them from working?

@khaeru khaeru added this to the 3.10 milestone Apr 17, 2024
@daymontas1
Copy link
Contributor Author

daymontas1 commented May 3, 2024

@glatterf42 The error arises because the tests cannot read ''solve_param'' when it is defined in ''MESSAGE-MACRO_run.gms''. As a result, an error occurs in ''model_solve'' since the solving mechanism is dictated by ''solve_param''. However, when ''solve_param'' is defined within ''macro_data_load.gms'', the tests are successful. Therefore, ''solve_param'' should be defined in ''macro_data_load.gms'' instead of ''MESSAGE-MACRO_run.gms''.

Copy link
Member

@glatterf42 glatterf42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the PR, looks good to me now! Please just rebase on top of main (or merge latest main into this branch) to enable merging this back to main :)

@khaeru
Copy link
Member

khaeru commented May 21, 2024

I've updated the PR description to re-add the checklist that appears in the PR template.

If, for a certain PR, one feels that certain items in the checklist are not necessary, use strikeout and put a note to explain why. In this case, however, I do think documentation and an entry in the release notes are needed.

If the branch contains "Merge …" commits, we use squash-and-merge; if it is rebased and these disappear, we can use an ordinary merge.

@glatterf42 glatterf42 modified the milestones: 3.10, 3.11 Jan 15, 2025
@khaeru khaeru self-assigned this Jan 15, 2025
Copy link
Member

@khaeru khaeru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glatterf42 and I discussed that I will take on the work to complete and merge this PR. Things required:

  • Tests of running MACRO in the new way.
    • Confirm the old and new methods give the same results.
  • Update documentation and release notes.
  • Provide a way to control the added setting directly via the Python API. This may involve changing it to be a compile-time or environment variable instead of a parameter; if so it may serve as reference for #451.

khaeru added a commit to daymontas1/message_ix that referenced this pull request Jan 20, 2025
khaeru added a commit to daymontas1/message_ix that referenced this pull request Jan 20, 2025
khaeru added a commit to daymontas1/message_ix that referenced this pull request Jan 21, 2025
@khaeru khaeru mentioned this pull request Jan 21, 2025
1 task
khaeru added a commit to daymontas1/message_ix that referenced this pull request Jan 21, 2025
khaeru added a commit to daymontas1/message_ix that referenced this pull request Jan 21, 2025
@khaeru
Copy link
Member

khaeru commented Jan 21, 2025

FYI @glatterf42 — following #909 and #910, this now appears to be working as intended.

khaeru added a commit to daymontas1/message_ix that referenced this pull request Jan 22, 2025
khaeru added a commit to daymontas1/message_ix that referenced this pull request Jan 22, 2025
khaeru added a commit to daymontas1/message_ix that referenced this pull request Feb 17, 2025
khaeru added a commit to daymontas1/message_ix that referenced this pull request Apr 3, 2025
@khaeru khaeru changed the title Adding the option to solve concurrently for all regions in MACRO Add an option to solve MACRO concurrently for all regions Apr 3, 2025
@khaeru khaeru added enh New features & functionality macro MACRO model or MESSAGE-MACRO link labels Apr 3, 2025
An if statement has been introduced to allow users to select whether they want to solve sequentially or concurrently for all regions. This is determined by the ''solve_param'' parameter, defined in ''MESSAGE-MACRO_run''. The default solving is sequential (for ''solve_param''=1), so if the user doesn't update the solving mechanism, the macro is solved as it used to be. In terms of efficiency, the concurrent solving is slightly more efficient. 

For the concurrent solving mechanism (for ''solve_param''=2), all nodes are activated from the beginning, thereby enabling simultaneous solving for all regions. This setting allows for factoring in interregional interactions into the optimization process. We want this concurrent optimization option, as we intend to incorporate interregional feedback mechanisms into MACRO, e.g., interregional investments. Given that in the current version of MACRO regions are independent, both solving mechanisms should produce the same results.
khaeru added a commit to daymontas1/message_ix that referenced this pull request May 26, 2025
khaeru added a commit to daymontas1/message_ix that referenced this pull request May 26, 2025
Copy link
Member

@khaeru khaeru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to merge once checks pass.

@khaeru khaeru merged commit 1f32d58 into iiasa:main May 26, 2025
65 of 67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enh New features & functionality macro MACRO model or MESSAGE-MACRO link safe to test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants