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

CDR cleaning session #1154

Merged
merged 12 commits into from
Nov 24, 2023
Merged

Conversation

katarkow
Copy link
Contributor

@katarkow katarkow commented Jan 11, 2023

Purpose of this PR

In this PR, the 33_CDR module is refactored from several realizations (each corresponding to a combination of CDR options available in REMIND) to one realization with switches determining which CDR technologies are available. It makes our life easier when we add more technologies to the CDR module :)

The new realization is called portfolio and the switches to turn on/off the CDR options cm_33[option abbreviation], e.g, cm_33EW, cm_33DAC. A set of used CDR options te_used33 contains technologies determined by the switches and is used for calculations.

Removing vm_otherFEdemand

When we add more technologies to the CDR module, it becomes tricky to report the final energy for each CDR using just vm_otherFEdemand(ttot,all_regi,all_enty), since it lacks the technology dimension. vm_otherFEdemand is replaced by v33_FEdemand(ttot,all_regi,all_enty,all_enty,all_te), where all_te is a CDR technology. The second all_enty is the FE demand of a given technology, e.g., for DAC it would be heat for material recovery and electricity for ventilation. The first all_enty corresponds to FE carrier chosen to supply the FE energy demand, e.g., heat for DAC can be provided by gas, H2, wasted heat, or electricity.
FE mapping fe2cdr(all_enty,all_enty,all_te) has been added to increase the readability of the code.

equations.gms

All equations are collected in one structured file. Firstly, equations that consider more than one CDR option are listed. Then option-specific equations follow in the agreed order: capacity, emissions, energy demand, costs, and limits-related. Option-specific equations have names that follow the pattern: q33_[option abbreviation]_[descriptive enough name].

Enhanced weathering improvements

  • rockgrind has been renamed to weathering
  • rlf_cz33 (an rlf subset for the temperature grades for EW) was added to increase the readability of the equations
  • Timesteps for the rock decay have been revisited and corrected
  • Bugfix: when we switch between realizations (e.g. weathering is added later in time), weathering on fields became unbounded in step ttot - 1 (look this equation, spotted by Anne).

Type of change

  • Refactoring
  • Fundamental change
  • This change requires a documentation update

Checklist:

  • My code follows the coding etiquette
  • I have performed a self-review of my own code
  • Changes are commented, particularly in hard-to-understand areas
  • I have updated the in-code documentation
  • I have adjusted reporting where it was needed
  • All automated model tests pass (FAIL 0 in the output of make test)

Further information (optional):

  • Comparison of results: /p/tmp/katarkow/remind/final-final-cdr-portfolio/output/cs2_output

@katarkow katarkow added documentation Improvements or additions to documentation code cleaning Code that could/should be cleaned up labels Jan 11, 2023
modules/33_CDR/portfolio/bounds.gms Show resolved Hide resolved
modules/33_CDR/portfolio/declarations.gms Show resolved Hide resolved
@@ -855,6 +852,16 @@ parameter
;
cm_gdximport_target = 0; !! def = 0
*'
parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

Great to have all these parameters in one "section" here. Would it make sense to rename others also to cm_33*** to indicate that they are only used in module 33, e.g. cm_33gs_ew, cm_33LimRock? Or would this be against our coding etiquette? And what do you think about assembling also other relevant switches here, like cm_frac_CCS, cm_frac_NetNegEmi, c_ccsinjecratescen, c_ccscapratescen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fully agree with cm_33gs_ew, cm_33LimRock, as they're only used in the CDR module. However, I will change it in a separate PR. When it comes to cm_frac_CCS, cm_frac_NetNegEmi, c_ccsinjecratescen, c_ccscapratescen, since these are used in the core, I'd keep them as they are, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, agree to keep the names for those. It would be nice to have them all in one place close together, but this can be done in a separate PR

modules/33_CDR/portfolio/equations.gms Outdated Show resolved Hide resolved
modules/33_CDR/portfolio/equations.gms Outdated Show resolved Hide resolved
modules/33_CDR/portfolio/equations.gms Outdated Show resolved Hide resolved
modules/33_CDR/portfolio/equations.gms Outdated Show resolved Hide resolved
@katarkow katarkow marked this pull request as ready for review July 21, 2023 07:20
@@ -855,6 +852,16 @@ parameter
;
cm_gdximport_target = 0; !! def = 0
*'
parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

yes, agree to keep the names for those. It would be nice to have them all in one place close together, but this can be done in a separate PR

*** SOF ./modules/33_CDR/portfolio/bounds.gms

vm_emiCdr.fx(t,regi,emi)$(not sameas(emi,"co2")) = 0.0;
vm_emiCdr.l(t,regi,"co2")$(t.val ge 2025 AND cm_ccapturescen ne 2) = -sm_eps;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this eq now applies to all technologies including EW, does it still make sense to tie it to cm_ccapturescen?

@LaviniaBaumstark
Copy link
Member

what is the status of thsi PR? will you continue working on it?

@strefler
Copy link
Contributor

I think this one is ready, not sure why it is not yet merged

@katarkow
Copy link
Contributor Author

I didn't merge it before cause I was away for a month, but now I'm back and merging :)

@katarkow katarkow merged commit 494e2cf into remindmodel:develop Nov 24, 2023
2 checks passed
@katarkow katarkow deleted the cdr-cleaning-session branch November 24, 2023 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleaning Code that could/should be cleaned up documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants