Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding the option to solve concurrently for all regions in MACRO #808

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

daymontas1
Copy link

@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.

Introducing a parameter (''solve_param'') 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 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.
@CLAassistant
Copy link

CLAassistant commented Mar 25, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

codecov bot commented Mar 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.4%. Comparing base (c359123) to head (376e97f).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main    #808     +/-   ##
=======================================
- Coverage   95.4%   95.4%   -0.1%     
=======================================
  Files         46      46             
  Lines       4354    4354             
=======================================
- Hits        4156    4155      -1     
- Misses       198     199      +1     

see 1 file with indirect coverage changes

@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
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''.

Introducing a parameter (''solve_param'') 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.
Defining ''solve_param'' in ''MESSAGE-MACRO_run.gms'' creates a problem in testing. Therefore, ''solve_param'' is defined in ''macro_data_load.gms'' instead.
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants