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 biomass demand for bioplastics in 62_material #427

Merged
merged 15 commits into from
Sep 29, 2022

Conversation

deleip
Copy link
Contributor

@deleip deleip commented Aug 10, 2022

🐦 Purpose of this PR 🐦

  • This PR introduces biomass demand for bioplastic production in 62_material. Future bioplastic production is projected as logistic curve defined by historic production in 2020 and the target maximum production and midpoint of the logistic function (s62_max_dem_bioplastic and s62_midpoint_dem_bioplastic). As default s62_max_dem_bioplastic is set to zero. In that case bioplastic is not considered in overall material demand.

🔧 Checklist for PR creator 🔧

  • Low risk : Simple bugfixes (missing files, updated documentation, typos) or Start/output scripts
  • Medium risk : New realization / Changes to existing realization / Other changes which don't modify default.cfg
  • High risk : New input files (if cfg$input is changed in default.cfg) / Modification to core model (eg. changes in equations, calculations, introduction of new sets etc.) / Other changes in default.cfg
  • Providing additional information based on PR label
  • Low risk : No new model run needed.
  • Medium risk : Default run based on the current version of the fork from which PR is made
  • High risk
    • Default run from the current develop branch
    • Default run based on the current version of the fork from which PR is made
  • 📉 Performance loss/gain from current default behavior 📈

    • Current develop branch default : 21 mins
    • This PR's default : 24 mins
  • Added changes to CHANGELOG.md

  • Compilation check (model starts without compilation errors - use gams main.gms action=c in model folder for testing).

  • No hard coded numbers and cluster/country/region names.

  • The new code doesn't contain declared but unused parameters or variables.

  • Where relevant, In-code comments added including documentation comments.

  • Made sure that documentation created with goxygen is okay (use goxygen::goxygen() for testing).

  • Changes to magpie4 R library for post processing of model output (ideally backward compatible).

  • Self-review of my own code.

  • In case of updated cellular input tgz file in default.cfg: scenario_config.csv has been updated accordingly (rcp1p9, rcp2p6 etc)

  • For high risk runs: validation of major model indicators - Land-use, emissions, food prices, Tau. %Delete this line in case it is not a high risk run%

Results of bioplastic scenario (with max. demand 400 mio. t in 2050) compared to default:
cropProduction
employment
foodExpenditureIndex
material_demand

⚠️ Additional notes or warnings ⚠️

NA

🚨 Checklist for RSE reviewer 🚨

  • PR is labeled correctly.
  • CHANGELOG is updated correctly
  • No hard coded numbers and cluster/country/region names.
  • No unnecessary increase in module interfaces
  • All required updates in interfaces (if any) have been properly adressed in the module contracts
  • In-code comments and documentation comments are satisfactory.
  • model behavior/performance is satisfactory.
  • Requested changes (if any) were applied correctly

🚨 Checklist for MAgPIE reviewer 🚨

  • PR is labeled correctly.
  • CHANGELOG is updated correctly
  • No hard coded numbers and cluster/country/region names.
  • Changes to the model are scientifically sound
  • In-code comments and documentation comments are satisfactory.
  • model behavior/performance is satisfactory.
  • Requested changes (if any) were applied correctly

@deleip deleip added the High risk Higher risk label Aug 10, 2022
@deleip deleip marked this pull request as draft August 10, 2022 17:29
Copy link
Member

@bodirsky bodirsky left a comment

Choose a reason for hiding this comment

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

looks good, but a few naming convention issues, and biomass demand should be the total biomass, not just the one without the histprical quantity (what the model is doing is correct, but the output variabels should be close to the values that people want to use when writing a paper, so it should be total quantity)

modules/62_material/exo_flexreg_apr16/declarations.gms Outdated Show resolved Hide resolved
modules/62_material/exo_flexreg_apr16/declarations.gms Outdated Show resolved Hide resolved
modules/62_material/exo_flexreg_apr16/declarations.gms Outdated Show resolved Hide resolved
modules/62_material/exo_flexreg_apr16/declarations.gms Outdated Show resolved Hide resolved
modules/62_material/exo_flexreg_apr16/declarations.gms Outdated Show resolved Hide resolved
modules/62_material/exo_flexreg_apr16/input.gms Outdated Show resolved Hide resolved
modules/62_material/exo_flexreg_apr16/preloop.gms Outdated Show resolved Hide resolved
modules/62_material/exo_flexreg_apr16/presolve.gms Outdated Show resolved Hide resolved
@@ -10,3 +10,22 @@ table f62_dem_material(t_all,i,kall) Historical material demand (mio. tDM)
$ondelim
$include "./modules/62_material/input/f62_dem_material.cs3"
$offdelim;

parameter f62_bioplastic2biomass(kall) Biomass demand for one unit of bioplastics (mio. tDM)
Copy link
Member

Choose a reason for hiding this comment

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

it should be named biomass2bioplastic, because it is biomass/bioplastic, e.g. 2to1, 2:1, 2/1, not vice versa.
I would even call it
f62_biomass2bioplastic_conversion_ratio
The unit seems wrong, shoudl be tDM biomass/tDM bioplastic (for mathematicians this seems as it cancels out, but it doesnt because its strictly speaking two different types of tons)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true... unfortunately this is also wrong in the input file name - I guess it would be worth doing a new pre-processing revision for this, or should I only change the parameter name?

Copy link
Member

Choose a reason for hiding this comment

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

ah damn... a lot of effort for a single parameter, maybe just include it in one of your next PRs which requires new preprocessing...

modules/62_material/module.gms Outdated Show resolved Hide resolved
@deleip deleip added enhancement New feature or request Medium risk and removed High risk Higher risk labels Sep 26, 2022
@deleip deleip marked this pull request as ready for review September 27, 2022 07:55
Copy link
Member

@tscheypidi tscheypidi left a comment

Choose a reason for hiding this comment

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

Looks good overall, mostly just some suggestions for improvements from my side. Only thing which I don't understand is why there is a special treatment for the case s62_max_dem_bioplastic = 0 and why this is happening in presolve, while the treatment of all other cases is in preloop. This should be clarified and ideally simplified.

modules/62_material/exo_flexreg_apr16/equations.gms Outdated Show resolved Hide resolved
p62_dem_food_lh(i) Food demand in last historical timestep (mio. tDM per yr)
p62_scaling_factor(i) Scaling factor for material demand (1)
p62_dem_material_lastcalibyear(i,kall) Material demand in last historical timestep (mio. tDM per yr)
p62_dem_food_lastcalibyearh(i) Food demand in last historical timestep (mio. tDM per yr)
Copy link
Member

Choose a reason for hiding this comment

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

perhaps use ref instead of lastcalibyear(h) as it is basically your point of reference for the following calculations, right?

modules/62_material/exo_flexreg_apr16/postsolve.gms Outdated Show resolved Hide resolved
modules/62_material/exo_flexreg_apr16/preloop.gms Outdated Show resolved Hide resolved
modules/62_material/exo_flexreg_apr16/preloop.gms Outdated Show resolved Hide resolved

*' @stop

* if max. bioplastic demand is set to zero, overwrite bioplastic demand with zero
if (s62_max_dem_bioplastic = 0,
Copy link
Member

Choose a reason for hiding this comment

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

why is this case treated separately and why in presolve and not in preloop where it is done for all other cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was left over from an earlier version of handling the double counted bioplastic substrate. I moved it to preloop

@deleip deleip merged commit 0fc4094 into magpiemodel:develop Sep 29, 2022
@deleip deleip deleted the f_bioplastic branch November 2, 2023 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Medium risk
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants