Closed macflo8 closed 2 months ago
Attention: Patch coverage is 29.32166%
with 323 lines
in your changes missing coverage. Please review.
Project coverage is 51.9%. Comparing base (
229eecb
) to head (4814e7a
).
* Files like message_ix_models/data/material/iea_mappings/*.csv: * should be stored as text, not using Git LFS. * contain the same phrase "iea_mappings" in their path and filename; this could be simplified. * do not appear to be mentioned anywhere in the documentation.
At the moment our biggest csv file is ~400 KB. Is it okay to store all of them as text?
* `message_data.tools.utilities.get_optimization_years()` does the same thing as [`ScenarioInfo.Y`](https://docs.messageix.org/projects/models/en/latest/api/util.html#message_ix_models.util.scenarioinfo.ScenarioInfo.Y). The latter is computed only once, and has tests and documentation. The former is not tested, and hits the database every time it is called. I would prefer that we take the opportunity, when moving code to `message_ix_models`, to use the better-maintained solution. Similar applies to `get_historical_years()`.
Just to be clear: This means we should refactor the functions that use these helpers and pass a ScenarioInfo
instance to them instead, correct?
At the moment our biggest csv file is ~400 KB. Is it okay to store all of them as text?
Generally yes. Git will automatically compress these files. Some further nuances:
foo bar --option=X
"), and will only ever be entirely replaced, then diffs would not be needed or very useful, and Git LFS can be used.Just to be clear: This means we should refactor the functions that use these helpers and pass a ScenarioInfo instance to them instead, correct?
model_years = ScenarioInfo(scen).Y
This is already an improvement; it will later make it easier to see where the ScenarioInfo object can be passed/used instead of the scenario itself. Since we may want to provide tested and documented alternatives for the codes in .util.compat.message_data
, let's not invest too much time in tinkering with them.
Okay thanks for confirming! I pushed the suggested changes. I think all the requested changes and comments are addressed now.
message_data
dependencies (some.yaml
and.csv
input files).utils/compat/message_data
.material-ix
grouputils/compat/message_data
to using tested equivalents ofmessage-ix-models
click
options frommaterial-ix
commands that are already implemented inmessage_ix_models/cli.py
Details: Building MESSAGEix-Materials on an existing MESSAGEix-GLOBIOM scenario still requires a few external data file that have previously been stored in a private repository. The respective files are migrated to
message-ix-models/data
and data paths in the modules updated accordingly in this PR. The files located inutil/compat/message_data
migrated in the course of #188 contain some incorrect import statements, which are fixed in this PR.The following files are needed in the build and were therefore added:
The build however still requires proprietary data from IEA Extended Energy Balances to calibrate historical industry activity. The path to the required file can be specified with a newly added CLI option
--iea_data_path
in thebuild
command.All input data for MESSAGEix-Materials is now packaged in a
.tar.gz
file, that can be fetched withmix-models fetch MESSAGEix-Materials
.When executing
mix-models material-ix --help
only the basic comments related to building/solving/reporting are shownHow to review
Run build/solve/report with
material-ix
CLI commands and confirm correct execution of each task.PR checklist
[ ] Continuous integration checks all ✅Not possible due to missing tests[ ] Add or expand tests; coverage checks both ✅will be handled via https://github.com/iiasa/message-ix-models/issues/194