Skip to content

Conversation

@macflo8
Copy link
Contributor

@macflo8 macflo8 commented Aug 8, 2025

This PR adds an improved industry reporter for MESSAGEix-Materials developed in the course of the ScenarioMIP project. It cherry-picks some initial commits from the #235 branch.

How to review

  1. Read the diff and note that the CI checks all pass.
  2. Look at the materials doc in the ReadTheDocs preview build of the documentation —in particular, the entry in “What's New” and linked entries.
  3. Ensure that changes/additions are self-documenting, i.e. that another
    developer (someone like the reviewer) will be able to understand what the code
    does in the future.

PR checklist

  • Continuous integration checks all ✅
  • Add or expand tests; coverage checks both ✅
  • Add, expand, or update documentation.
  • Update doc/whatsnew.

@macflo8 macflo8 self-assigned this Aug 8, 2025
@macflo8 macflo8 added material MESSAGEix-Materials variant p:SSP-2024 2024 SSP updates and ScenarioMIP enh New features or functionality labels Aug 8, 2025
@codecov
Copy link

codecov bot commented Aug 8, 2025

Codecov Report

❌ Patch coverage is 75.12195% with 102 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.4%. Comparing base (e45db65) to head (780c8f4).
⚠️ Report is 48 commits behind head on main.

Files with missing lines Patch % Lines
...e_ix_models/model/material/report/run_reporting.py 53.3% 91 Missing ⚠️
...ssage_ix_models/model/material/report/reporting.py 64.2% 10 Missing ⚠️
...models/tests/model/material/test_reporter_utils.py 95.8% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main    #395     +/-   ##
=======================================
- Coverage   77.1%   73.4%   -3.8%     
=======================================
  Files        250     257      +7     
  Lines      20198   21451   +1253     
=======================================
+ Hits       15589   15754    +165     
- Misses      4609    5697   +1088     
Files with missing lines Coverage Δ
message_ix_models/model/material/report/config.py 100.0% <100.0%> (ø)
..._ix_models/model/material/report/reporter_utils.py 100.0% <100.0%> (ø)
message_ix_models/testing/__init__.py 80.6% <100.0%> (-14.2%) ⬇️
...sage_ix_models/tests/model/material/test_report.py 100.0% <100.0%> (ø)
..._models/tests/model/material/test_run_reporting.py 100.0% <100.0%> (ø)
message_ix_models/tests/test_util.py 100.0% <100.0%> (ø)
...models/tests/model/material/test_reporter_utils.py 95.8% <95.8%> (ø)
...ssage_ix_models/model/material/report/reporting.py 3.5% <64.2%> (ø)
...e_ix_models/model/material/report/run_reporting.py 53.3% <53.3%> (ø)

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@macflo8 macflo8 force-pushed the ssp-dev-industry-reporter branch 3 times, most recently from f6a03de to fb91fd5 Compare August 8, 2025 13:16
@macflo8 macflo8 force-pushed the ssp-dev-industry-reporter branch from 04df012 to e103f06 Compare August 18, 2025 14:57
@macflo8 macflo8 marked this pull request as ready for review August 21, 2025 08:21
macflo8 and others added 20 commits August 21, 2025 11:04
Remove unnecessary reads/writes to disk
Replace sequential unit conversions with loop
- Add aggregation step to Final Energy|Industry
- Fix |Solids, |Coal and |Biomass aggregate components
- Add missing ammonia mass-energy conversion
- Fix unit conversion for Final Energy quantities
Only replace Liquids|Other and not Other Sector strings
- Move electricity mapping
- Rename function variables for readability
Drop "mode" column in genno Reporter

Needs to be dropped to allow for aggregation of methanol production with
new meth_h2 modes
- Add Solar to Other Sector total
- Add other chemicals to aggregates
- Add missing FE HVC total
@macflo8 macflo8 force-pushed the ssp-dev-industry-reporter branch from ffcd011 to c35c835 Compare September 2, 2025 08:10
macflo8 and others added 6 commits September 2, 2025 10:26
Use "var: in" to align with corresponding basic config file.
- Rename ReporterConfig → Config, move class to its own submodule
- Use stdlib dataclasses instead of pydantic.
- load_config() wraps Config.from_files().
- Move .reporter_utils.create_var_map_from_yaml_dict()  →
  Config.use_vars_dict()
- Move .run_reporting.create_agg_var_map_from_yaml() →
  Config.use_aggregates_dict().
- Add missing material/report/__init__.py.
- Remove pydantic from optional dependencies.


@dataclass
class Config:
Copy link
Member

@khaeru khaeru Sep 2, 2025

Choose a reason for hiding this comment

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

@macflo8 this class replaces the pydantic.BaseModel subclass. As well as doing that:

  • I collected configuration-handling logic together with the class. This includes:
    • Logic formerly in load_config() for locating and handling the format of configuration files.
    • Two functions in different modules: reporter_utils.create_var_map_from_yaml_dict() and run_reporting.create_agg_var_map_from_yaml(). These are now closely located with the Config.mapping attribute they are meant to update.
  • I added docstrings about the specific data structures that the code expects/can handle, with examples. These should help to maintain the YAML file collection.

test_load_config() still passes; I will monitor CI to see if anything broke. I will also watch for coverage gaps and push ~1 more commit if needed to address those.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! The new test_run() integration tests marked with XFAIL still needs some adjustment. So that one is failing atm. Since the function errors out early, it does not cover all remaining lines of run_reporting yet. I need to see if it is possible to reach the codecov target with a few smalll changes.

Copy link
Member

Choose a reason for hiding this comment

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

I removed the raises=… keyword from this mark; this way the mark matches regardless of the type of exception raised.

@khaeru khaeru force-pushed the ssp-dev-industry-reporter branch from b16c780 to 7bf714d Compare September 2, 2025 15:51
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.

Okay, with a few additional test stubs we are just shy of the Codecov thresholds. I don't see us getting much further without a push to build out more complete fixtures, and we've agreed that is out of scope for this PR.

From my POV then all 3 things from my last review are addressed and this is good to merge. Please let me know if you have anything further to push, otherwise I'll merge.

@macflo8
Copy link
Contributor Author

macflo8 commented Sep 2, 2025

Thanks for your contributions and help! I tried to address all other suggestions except for 2 who are not marked as resolved yet. In my view, they can also be tackled in a follow-up PR. I will push one more commit, updating the material docs, since pydantic is still mentioned there and then do the merge.

@khaeru
Copy link
Member

khaeru commented Sep 3, 2025

The two failing CI jobs appear to be flaky (I have seen similar failures unrelated to the current PR/changes, and they occur only on one OS and not others), so I will merge the PR without waiting for them to complete.

@khaeru khaeru merged commit 0e5fb53 into main Sep 3, 2025
40 of 46 checks passed
@khaeru khaeru deleted the ssp-dev-industry-reporter branch September 3, 2025 08:02
@khaeru khaeru added this to the 2025-09 milestone Sep 9, 2025
hyyyyc pushed a commit to hyyyyc/message-ix-models that referenced this pull request Sep 24, 2025
Wegatriespython pushed a commit to Wegatriespython/message-ix-models that referenced this pull request Oct 6, 2025
junukitashepard pushed a commit to junukitashepard/message-ix-models that referenced this pull request Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enh New features or functionality material MESSAGEix-Materials variant p:SSP-2024 2024 SSP updates and ScenarioMIP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants