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

Read GAMS CPLEX solver options for MESSAGE from user config #557

Merged
merged 12 commits into from
Feb 23, 2022

Conversation

khaeru
Copy link
Member

@khaeru khaeru commented Feb 7, 2022

This file adds a new recognized key, "message model options", in the user configuration file (~/.local/share/ixmp/config.json or similar), that corresponds to passing options to Scenario.solve(…) (docs).

So, for instance, with the following in config.json:

{
  "my-platform": {"backend": "jdbc", "etc": "etc"},
  "...": "...",
  "message solve options": {"lpmethod": 4},
}

…then:

s.solve()

is updated to have the same effect as:

s.solve(solve_options={"lpmethod": 4})

Also:

How to review

  • Read the diff and note that the CI checks all pass.

PR checklist

  • Open a corresponding PR upstream (in ixmp) to address the FIXME in the added comment → Simplify & improve configuration ixmp#435.
  • Continuous integration checks all ✅
  • Add or expand tests; coverage checks both ✅
  • Add, expand, or update documentation.
  • Update release notes.

@khaeru khaeru added the enh New features & functionality label Feb 7, 2022
@khaeru khaeru self-assigned this Feb 7, 2022
@khaeru
Copy link
Member Author

khaeru commented Feb 7, 2022

FYI @gidden @OFR-IIASA

@codecov
Copy link

codecov bot commented Feb 7, 2022

Codecov Report

Merging #557 (1bde1bf) into main (1df3619) will decrease coverage by 1.3%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##            main    #557     +/-   ##
=======================================
- Coverage   93.8%   92.5%   -1.4%     
=======================================
  Files         40      41      +1     
  Lines       3057    3153     +96     
=======================================
+ Hits        2870    2919     +49     
- Misses       187     234     +47     
Impacted Files Coverage Δ
message_ix/__init__.py
message_ix/models.py
message_ix/tests/test_tutorials.py
message_ix/tests/test_legacy_version.py
message_ix/cli.py
message_ix/reporting/pyam.py
message_ix/tests/test_feature_price_commodity.py
message_ix/tests/test_reporting.py
message_ix/tests/test_feature_duration_time.py
message_ix/util/tutorial.py
... and 71 more

@gidden
Copy link
Member

gidden commented Feb 7, 2022

Thanks @khaeru! Seems like the right solution.

@khaeru khaeru changed the title Read GAMS CPLEX solver options for message from user config Read GAMS CPLEX solver options for MESSAGE from user config Feb 9, 2022
@khaeru
Copy link
Member Author

khaeru commented Feb 10, 2022

This is also affected by #559.

@LauWien LauWien mentioned this pull request Feb 23, 2022
1 task
@LauWien LauWien linked an issue Feb 23, 2022 that may be closed by this pull request
3 tasks
@LauWien LauWien added this to the 3.5 milestone Feb 23, 2022
@khaeru
Copy link
Member Author

khaeru commented Feb 23, 2022

So @LauWien if I understand the description at #566 (comment), then the issue in .tools.add_year (#559) should go away now that pandas 1.4.1 (I guess, different from 1.4.0) is used and we are no longer using Python 3.6?

@khaeru
Copy link
Member Author

khaeru commented Feb 23, 2022

It looks like there were several regressions related to pd.DataFrame.loc fixed in pandas 1.4.1: https://pandas.pydata.org/pandas-docs/stable/whatsnew/v1.4.1.html#fixed-regressions. Maybe one of them is the one we were seeing 🤞🏾

@LauWien
Copy link
Contributor

LauWien commented Feb 23, 2022

So @LauWien if I understand the description at #566 (comment), then the issue in .tools.add_year (#559) should go away now that pandas 1.4.1 (I guess, different from 1.4.0) is used and we are no longer using Python 3.6?

Yes, I think so. I ran the tests here now, and "only" the test test_tutorial[R_austria] is failing on ubuntu-latest so far. Lets see 🙈 :D

@LauWien
Copy link
Contributor

LauWien commented Feb 23, 2022

test_tutorial[R_austria] is failing on ubuntu-latest so far

Just a "time out" error.

E           nbclient.exceptions.CellTimeoutError: A cell timed out while it was being executed, after 10 seconds.
E           The message was: Cell execution timed out.
E           Here is a preview of the cell contents:
E           -------------------
E           year_pairs = as.matrix(expand.grid(horizon,horizon) %>%
E                                    rowwise() %>%
E                                    filter(Var2 >= Var1) %>%
E                                    arrange(Var1))
E           vintage_years <- year_pairs[,1]
E           act_years <- year_pairs[,1]
E           -------------------

@khaeru
Copy link
Member Author

khaeru commented Feb 23, 2022

Just a "time out" error.

A little weird as (1) it's occurring on all Python versions and (2) that R code isn't very complex.

@khaeru
Copy link
Member Author

khaeru commented Feb 23, 2022

  • Add, expand, or update documentation.
  • Update release notes.

I still need to do these items, so will try them now and then see if the Jupyter R test still fails when I push again.

@khaeru
Copy link
Member Author

khaeru commented Feb 23, 2022

see if the Jupyter R test still fails

It did. In 1bde1bf I added an exclusion, replacing an older one that is no longer necessary because we no longer test on Python 3.6.

@khaeru khaeru force-pushed the feature/solve-args-config branch from 64b13f2 to 1bde1bf Compare February 23, 2022 11:21
@khaeru khaeru requested a review from LauWien February 23, 2022 11:22
@khaeru
Copy link
Member Author

khaeru commented Feb 23, 2022

If checks pass, please approve and merge using a merge commit.

@LauWien
Copy link
Contributor

LauWien commented Feb 23, 2022

If checks pass, please approve and merge using a merge commit.

Will do :) Many thanks!

@LauWien
Copy link
Contributor

LauWien commented Feb 23, 2022

If checks pass

Hm, the coverage will decrease by -1.4%.

@khaeru
Copy link
Member Author

khaeru commented Feb 23, 2022

Codecov seems inconsistent, e.g. if we look here https://app.codecov.io/gh/iiasa/message_ix/branch/feature%2Fsolve-args-config —the number of missed lines is still 187, different from the comment above.

Sometimes this takes a few seconds or minutes to get updated.

@@ -144,6 +143,6 @@ jobs:
run: make --directory=doc html

- name: Upload test coverage to Codecov.io
uses: codecov/codecov-action@v1.2.1
uses: codecov/codecov-action@v2
Copy link
Member Author

@khaeru khaeru Feb 23, 2022

Choose a reason for hiding this comment

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

@LauWien the discrepancy might also be due to this update to a newer version of the Codecov action.

Compare

The difference of 47 lines corresponds to .testing.nightly, so I think this might resolve itself at the next nightly test run.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, yes, I'll then merge!

@LauWien LauWien merged commit e2ac6ff into main Feb 23, 2022
@khaeru khaeru deleted the feature/solve-args-config branch February 23, 2022 12:20
khaeru added a commit to iiasa/message-ix-models that referenced this pull request Feb 23, 2022
khaeru added a commit to iiasa/message-ix-models that referenced this pull request Feb 23, 2022
khaeru added a commit to iiasa/message-ix-models that referenced this pull request Feb 23, 2022
khaeru added a commit to iiasa/message-ix-models that referenced this pull request Feb 23, 2022
glatterf42 pushed a commit to iiasa/message-ix-models that referenced this pull request Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enh New features & functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ValueError in .add_year.interpolate_2d() with pandas 1.4.0 Include Python version 3.10 and remove 3.6 in CI
3 participants