Skip to content

Conversation

ywpratama
Copy link
Contributor

This draft PR adds DACCS and modifies the CCS infrastructure representation in MESSAGEix for ScenarioMIP.
It includes a set of functions to perform the task, as well as the necessary data provided in YAML files.
Currently, several functions still need docstrings for proper documentation.

How to review

  • Read the diff and verify that all CI checks pass.
  • Ensure that all new functions have proper docstrings explaining their purpose.

PR checklist

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

@ywpratama ywpratama requested a review from khaeru September 9, 2025 08:39
@ywpratama ywpratama self-assigned this Sep 9, 2025
@ywpratama ywpratama added enh New features or functionality p:SSP-2024 2024 SSP updates and ScenarioMIP novel-cdr MESSAGEix-CDR variant labels Sep 9, 2025
Copy link

codecov bot commented Sep 9, 2025

Codecov Report

❌ Patch coverage is 0% with 133 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.1%. Comparing base (fc71e2b) to head (5f11cd7).

Files with missing lines Patch % Lines
message_ix_models/tools/add_dac/__init__.py 0.0% 133 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main    #419     +/-   ##
=======================================
- Coverage   74.2%   73.1%   -1.2%     
=======================================
  Files        260     261      +1     
  Lines      21818   21951    +133     
=======================================
- Hits       16198   16050    -148     
- Misses      5620    5901    +281     
Files with missing lines Coverage Δ
message_ix_models/tools/add_dac/__init__.py 0.0% <0.0%> (ø)

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

@khaeru
Copy link
Member

khaeru commented Sep 9, 2025

Thanks for starting this draft!

Some quick reactions—these may be off the mark, since I am just guessing by looking at the Git history and changes:

  • To make work easier, it will be good to have at least 1 minimal test that runs the added code—even if the outcome of that test is an XFAIL. It seems one (the main?) entry-point is .project.ssp.script.util.functions.add_ccs_setup(). If that's accurate, a minimal test could simply call this function with an empty Scenario or bare RES scenario. A slightly better test would add some contents to that empty/bare scenario so that the function runs without error, or at least partway through. See Add GEIDCO IV extended features and documentation #383 where @yiyi1991 did something similar.

  • With such a test in place, it will be easier to address the code quality failures, particularly the ones about excessive complexity in add_ccs_setup() and .tools.add_dac.generate_df().

  • I see there are a number of large YAML files with mixed configuration/data, and that these eventually go into .tools.add_dac.add_tech(). Like in Add new industry reporter #395 (@macflo8's PR) and again Add GEIDCO IV extended features and documentation #383, it will be good to have some code or a class that reads these files, separate from where they are used. The reason for this is to distinguish and clarify 2 things:

    1. The particular layout of these files/their format, and
    2. What kind of information the code (e.g. add_tech()) needs.

    Having these 2 things described would make it more realistic to re-use the code by supplying different information (2) and for others to modify the files (or derive new files) and ensure they contain the needed information in a valid format (1). It will also help towards simplifying add_tech() by separating config-/file-parsing logic from the model-building logic.

  • I am not clear what the distinction is between:

    • Generic capability for adding DACCS representation to MESSAGEix-GLOBIOM family models.
    • A “standard representation” (if there is one) of DACCS in the family—i.e. is this the main/preferred/only way to do it, or might there be others?
    • The particular application or parametrization (numbers) used with the above in the context of the ScenarioMIP project.
    • A standard parametrization, e.g. if someone is creating a SSP1-based scenario with DACCS outside of the ScenarioMIP project, which data/config they should use, and whether this is distinct from that used within ScenarioMIP.

    I think we can clarify these via some discussion, that could then be summarized in documentation. They may also lead us to rename/move code, e.g. .tools.add_dac might be more properly named .model.dac if it contains a ‘standard’ representation that should always be used with MESSAGEix-GLOBIOM (just as .model.material contains the ‘standard’ way of representing materials and industry, etc.)

Of course, I'm happy to help or bring in colleagues on any or all of these points.

@Tyler-lc
Copy link

Tyler-lc commented Sep 9, 2025

Thanks for the PR on the DAC and CCS :)
Right now there is a broken link between the h2_elec and meth_h2.
As of now h2_elec has three modes: M1, feedstock and fuel.
The addon_conversion technology that connects the electrolyzers to meth_h2 has also the same three modes
The issue is that meth_h2 takes carbon from three different sources: fic, dac, bic.
So in meth_h2 we have the following modes: feedstock_bic, feedstock_fic, feedstock_dac, fuel_bic, fuel_fic, fuel_dac.
Basically right now, because the different naming structure in the modes of h2_elec, addon_conversion and meth_h2, the two technologies are not connected.
Somewhere in between this linkage broke.
@macflo8 Should know a little bit more on the topic. We have discussed it in person already (also he brought this PR to my attention).

@khaeru khaeru added this to the 2025-10 milestone Sep 9, 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 novel-cdr MESSAGEix-CDR variant p:SSP-2024 2024 SSP updates and ScenarioMIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants