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

Peatland Module + SSPDB input bugfix #178

Merged
merged 84 commits into from
May 13, 2020
Merged

Conversation

flohump
Copy link
Contributor

@flohump flohump commented Apr 24, 2020

Peatland Module + input files
new mapping in climate module
SSPDB input bugfix (GHG prices and Bioenergie demand updated to most recent snapshot)

@tscheypidi
Copy link
Member

Please add some info about your changes to the changelog.md

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.

did not finish my review yet, but I think the set and equation structure need some imoprovements first

modules/58_peatland/on/declarations.gms Show resolved Hide resolved
modules/58_peatland/on/sets.gms Show resolved Hide resolved
modules/58_peatland/on/sets.gms Outdated Show resolved Hide resolved
modules/58_peatland/on/sets.gms Outdated Show resolved Hide resolved
Copy link
Member

@k4rst3ns k4rst3ns left a comment

Choose a reason for hiding this comment

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

Sorry I am still struggling with some equations and my understand of it.
Specially the scaling factor is still not to clear to me.

modules/45_climate/static/sets.gms Show resolved Hide resolved
vm_peatland_emis(j2) *
s56_peatland_policy *
sum((ct,cell(i2,j2)),
im_pollutant_prices(ct,i2,"co2_c")*12/44
Copy link
Member

Choose a reason for hiding this comment

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

The '12/44' confused me here. It is probably right, but maybe transform vm_peatland_emis into correct units before entering this equation. No other equation needs unit transformation in this module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The unit of vm_peatland_emis is t CO2eq per year. Since im_pollutant_prices is in USD/tC, I have to convert the C price into CO2 price.
If I do the conversion in the peatland module, vm_peatland_emis would have the unit t Ceq per year. This unit seems strange to me. At least I have never seen it anywhere before.

parameters
p58_scaling_factor(j) Scaling factor for managed peatland (1)
p58_intact_ratio(t,j) Ratio of intact and total peatland (1)
p58_peatland_degrad_used(j) Helper for peatland initialization (mio. ha)
Copy link
Member

Choose a reason for hiding this comment

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

maybe for clarity more meaningful names for the 'Helper for peatland initialization'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Intermediate calculation in peatland initialization (mio. ha)
Weight for intermediate calculation in peatland initialization (1)
Better?

modules/58_peatland/on/preloop.gms Outdated Show resolved Hide resolved
modules/58_peatland/on/presolve.gms Show resolved Hide resolved
modules/58_peatland/on/presolve.gms Outdated Show resolved Hide resolved
modules/58_peatland/on/presolve.gms Show resolved Hide resolved
modules/58_peatland/on/equations.gms Show resolved Hide resolved
modules/58_peatland/on/sets.gms Show resolved Hide resolved
modules/58_peatland/on/equations.gms Show resolved Hide resolved
Copy link
Member

@k4rst3ns k4rst3ns left a comment

Choose a reason for hiding this comment

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

Sorry, one idea that I forgot.

modules/58_peatland/on/presolve.gms Show resolved Hide resolved
@flohump flohump requested a review from tscheypidi May 8, 2020 11:56
Copy link
Member

@k4rst3ns k4rst3ns left a comment

Choose a reason for hiding this comment

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

I guess I finally understand the role of unused and I think after the adding of the missing constrains in the presolve (which you already did), you have a go from my side.
The understanding of the module (specifically the management set) is a bit complicated, but the explanation in the peatland paper works good. if it is published you will (and should) probably add the reference in the module file.

@flohump flohump dismissed tscheypidi’s stale review May 13, 2020 07:04

Clarified via personal communication

@flohump flohump merged commit 2baf48d into magpiemodel:develop May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants