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

Add precautionary checks to Model.initialize_items #315

Merged
merged 10 commits into from
Apr 30, 2020

Conversation

khaeru
Copy link
Member

@khaeru khaeru commented Apr 23, 2020

This PR adds changes necessary for iiasa/message_ix#190. In particular, it ensures that the Model.initialize() method is always called when a Scenario is cloned or loaded.

This helps in the following way:

Also:

  • Model.initialize_items() checks whether the returned object is in a checked-out state when it is received, and leaves the object in the same state.
  • Backend.init_ts() and Backend.init_s() are consolidated and simplified.
  • Scenario.__init__() now chains through its parent method, TimeSeries.__init__().
  • Made some follow-up changes to Add ixmp show-versions #320. test_show_versions would load pyam and message_data, which both change the pint 'application registry'; this would cause the ixmp reporting tests related to units to fail, depending on which subset of tests was run.

How to review

PR checklist

  • Tests added.
  • Documentation added.
  • Release notes updated.

@khaeru khaeru self-assigned this Apr 23, 2020
@khaeru khaeru marked this pull request as draft April 23, 2020 10:47
@codecov
Copy link

codecov bot commented Apr 26, 2020

Codecov Report

Merging #315 into master will increase coverage by 0.12%.
The diff coverage is 99.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #315      +/-   ##
==========================================
+ Coverage   97.43%   97.56%   +0.12%     
==========================================
  Files          41       41              
  Lines        4292     4353      +61     
==========================================
+ Hits         4182     4247      +65     
+ Misses        110      106       -4     
Impacted Files Coverage Δ
ixmp/model/gams.py 97.67% <ø> (+4.34%) ⬆️
ixmp/core.py 95.63% <95.45%> (-0.18%) ⬇️
ixmp/backend/base.py 98.63% <100.00%> (-0.02%) ⬇️
ixmp/backend/jdbc.py 95.17% <100.00%> (+0.44%) ⬆️
ixmp/model/base.py 100.00% <100.00%> (ø)
ixmp/model/dantzig.py 100.00% <100.00%> (+4.76%) ⬆️
ixmp/reporting/utils.py 95.83% <100.00%> (+0.18%) ⬆️
ixmp/testing.py 94.15% <100.00%> (+0.23%) ⬆️
ixmp/tests/backend/test_base.py 98.27% <100.00%> (-0.03%) ⬇️
ixmp/tests/reporting/test_computations.py 100.00% <100.00%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd7f77b...4ddee80. Read the comment docs.

@khaeru khaeru marked this pull request as ready for review April 29, 2020 15:01
- Ensures tests pass no matter which subset/order is run.
- Also tidy reporting.utils.parse_units.
khaeru added a commit to behnam-zakeri/message_ix that referenced this pull request Apr 29, 2020
- Ensures storage-related parameters are added to any Scenario loaded;
  this allows test_legacy_scenario to pass.
khaeru added a commit to behnam-zakeri/message_ix that referenced this pull request Apr 29, 2020
- Ensures storage-related parameters are added to any Scenario loaded;
  this allows test_legacy_scenario to pass.
khaeru added a commit to behnam-zakeri/message_ix that referenced this pull request Apr 30, 2020
- Ensures storage-related parameters are added to any Scenario loaded;
  this allows test_legacy_scenario to pass.
@khaeru khaeru force-pushed the feature/careful-init branch 2 times, most recently from 2ac3173 to 0112f33 Compare April 30, 2020 09:17
@behnam-zakeri
Copy link
Contributor

Thanks @khaeru for imprving the code to account for temporary and/or new items introduced in a development branch. I see this very useful, as the user can add these new/temporary items in one place message_ix.models.py and make sure that the entire ixmp-message_ix ecosystem will handle these items as expected. I tested this in combination with #190 in message_ix and it works well.

@khaeru khaeru merged commit 91136c8 into iiasa:master Apr 30, 2020
@khaeru khaeru deleted the feature/careful-init branch April 30, 2020 10:32
khaeru added a commit to iiasa/message_ix that referenced this pull request Apr 30, 2020
- Add mapping of time and period
- Update data load for storage mappings
- Update data load and parameter/sets def
- Update model core
- Add unit test and explanation
- Add a document for illustration
- Update and clean time mapping
- Update data load
- Update and clean model core
- Clean sets and parameter definition text
- Add to release notes
- Clean up text in GAMS
- Move storage init to core.py for use in all scenarios
- Cleanup of test_storage
- Remove commodity index from storage bounds and losses
- Add relation_storage as an option (storage_equality removed)
- Map storage_balance to map_tec_storage_level
- Add inline documentation to model_core
- Clean up GAMS files before rebasing
- Small modifications after rebasing
- Changes related to initialization of storage
- Clean up notation and complete documentation
- Improve tests to consider the initial value
- Improve mapping of storage level for storage with no loss
- Update asserts in test_reporting explicitly
- Cleanup of documentation and GAMS core
- Update test_legacy_version for changes in GAMS
- Improve text of test and documentation
- Consider lvl_temporal directly in parameter time_seq
- Make the connection between the last and first time steps (storage cycle)
- Remove second 'year' from index of time_period mapping
- Move init_storage to models.MESSAGE.initialize
- Minor code cleanup
- Use ixmp.Model.initialize_items()
- Update hard coded lengths in test_reporting and test_tutorials
- Rename test_storage and move to tests folder
- Accept and check an optional scheme kwarg to message_ix.Scenario
- Cleanup after rebase
  - Code removed by 56f7bd5
  - Code removed by 3bd21e5
- Restore line endings of period_parameter_assignment.gms
- Simplify the mapping of charge-discharge technologies to storage
- Remove map_time_period from include folder to data_load.gms
- Correct the mapping of initial storage content
- Change the sign of relation_storage and some text edits
- Remove new bound definitions
- Consider commodity in equations
- Correct linter errors
- Update period_parameter_assignment.gms
- Rename and cleanup storage equivalence equation
- Update reporting tests for new lengths
- Update Scenario constructor per iiasa/ixmp#315
  - Ensures storage-related parameters are added to any Scenario loaded;
    this allows test_legacy_scenario to pass.
- Rename storage.PNG to storage.png, consistent with usage in docs
- Correct ReST syntax for storage docs in model_core.gms
  - Reference label with '-' instead of spaces.
  - Underlines match heading length.
  - Space between paragraph text and '..'.
  - Correct relative path to figure storage.png.
- Update coding style guide for docs, GAMS code
- Update release notes with migration note; add storage items to docs
- Add autosummary to model docs
- Remove storage relations; to be modeled by existing parameters
- Update reporting length numbers
- References to message_ix in RELEASE_NOTES.rst
- Add documentation of storage params
- Change test assert values to pass CI
- Add documentation of sets for storage
- Edit nomenclature of storage equations
- Correct typo in COMMODITY_BALANCE equation
- Add one missing underline symbol in params section title
- Update test_reporting.py
- Edit text for parameters of storage
- Address Sphinx warnings

Co-authored-by: Matthew Gidden <[email protected]>
Co-authored-by: Paul Natsuo Kishimoto <[email protected]>
Co-authored-by: Francesco Lovat <[email protected]>
@khaeru khaeru mentioned this pull request Jul 22, 2021
7 tasks
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.

2 participants