-
Notifications
You must be signed in to change notification settings - Fork 159
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
Addition of irrigation water pumping costs for India #401
Addition of irrigation water pumping costs for India #401
Conversation
# | Contact: magpie@pik-potsdam.de | ||
|
||
name,type,reason | ||
vm_water_cost, variable, calculates cost of water pumping to be used in the costs module directly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the variable vm_water_cost
is declared in the water demand module. As it is an interface variable it has to be declared and set in every realization of that module. Currently, the other realizations won't work anymore due to a domain violation error (the costs module tries to access a variable which does not exist).
So, if you have a water demand realization which should not introduce any water costs, you still have to do two things:
- declare the variable
- fix the variable to 0
*Pumping cost in the current time step | ||
if(m_year(t) > s42_pumping_startyear, | ||
ic42_pumping_cost(i) = f42_pumping_cost(t,i)*s42_multiplier; | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens for years before the start year with ic42_pumping_costs
? The parameter should be initialized for these years as well. And do you actually use this feature that the start year can be changed? If not, I would suggest to skip it and just set the pumping costs for all years
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the initialization in presolve and also changed scalar description to clarify the purpose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here as in the other realization: s42_pumping = 0
does not deactivate pumping costs
scripts/start/projects/india_water.R
Outdated
# cfg$gms$s42_shock_year <- 2020 #def = 1995 | ||
# cfg$gms$s42_shock_scalar<- 0.4 #def = 0.4 | ||
|
||
#start_run(cfg, codeCheck=F) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please don't add uncommented code. If you want to have this code only sometimes activated, use a switch instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. I also changed the start script completely now as it had irrelevant material for my test runs. I have renamed it to fable_india
to also contain the input data files for project requirements.
remove mention of India input files and add them to separate start script Co-authored-by: Jan Dietrich <dietrich@pik-potsdam.de>
Bugfix in unit Co-authored-by: Jan Dietrich <dietrich@pik-potsdam.de>
…gpie into irrigationcosts
Thanks for the clarifications. My follow-up questions refer to
|
This is an important point Feli brought up and I somehow missed. I somehow was (apparently incorrectly) assuming that in a default setup we have globally 0 pumping costs and that only in your specific India setup you introduce costs for India. As it is crucial not to introduce systematic biases towards certain regions in a global analysis it is critically important that in the default configuration there are 0 pumping costs globally, including India. |
Hi Vartika, for our standard runs it is important to avoid systematic biases where possible, independent of whether they have a significant impact on the results or not. Hence, there must be a setting used as default in which all regions are treated equally (in this particular case a scenario in which all pumping costs are 0). |
Thank you all for the discussion on this. I have now made the change so that default pumping costs are 0 for all the regions and added the switch option in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes requested.
config/default.cfg
Outdated
cfg$gms$c41_initial_irrigation_area <- "LUH2v2" # def = LUH2v2 | ||
|
||
# * Sets the rate of depreciation of irrigation infrastructure in every timestep. | ||
# * Only applicable when (endo_apr13) realization is selected | ||
cfg$gms$s41_AEI_depreciation <- 0 # def = 0 | ||
|
||
|
||
# * switch for initialization area | ||
# * (LUH2v2): area equipped for irrigation based on LUH2v2 | ||
# * (Siebert): area equipped for irrigation from Siebert et al. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this moved to the end here and not before the line where this switch is used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a mistake. My mouse playing catch. Fixed it now.
@@ -15,15 +15,18 @@ parameters | |||
* country-specific scenario switch | |||
p42_country_dummy(iso) Dummy parameter indicating whether country is affected by EFP (1) | |||
p42_EFP_region_shr(t_all,i) Weighted share of region with regards to EFP (1) | |||
ic42_pumping_cost(i) Parameter to capture values for pumping costs in a particular time step (USD per m^3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here also instead of USD
write USD05MER
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
; | ||
|
||
positive variables | ||
vm_watdem(wat_dem,j) Water demand from different sectors (mio. m^3 per yr) | ||
v42_irrig_eff(j) Irrigation efficiency (1) | ||
vm_water_cost(i) Cost of irrigation water (USD per m^3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also here the unit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
*Costs of pumping are taken observed values in India and a conversion factor is applied to change them into USD values per cubic meter | ||
parameter | ||
f42_pumping_cost(t_all,i) Cost of pumping irrigation water (USD per m^3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -106,3 +108,13 @@ $ondelim | |||
$include "./modules/42_water_demand/input/f42_env_flow_policy.cs3" | |||
$offdelim | |||
; | |||
|
|||
*Costs of pumping are taken observed values in India and a conversion factor is applied to change them into USD values per cubic meter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here is a need for more documentation. Perhaps refer to your study in preparation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edited reference of forthcoming paper, god willing!
@@ -15,18 +15,18 @@ parameters | |||
* country-specific scenario switch | |||
p42_country_dummy(iso) Dummy parameter indicating whether country is affected by EFP (1) | |||
p42_EFP_region_shr(t_all,i) Weighted share of region with regards to EFP (1) | |||
ic42_pumping_cost(i) Parameter to capture values for pumping costs in a particular time step (USD per m^3 per yr) | |||
ic42_pumping_cost(i) Parameter to capture values for pumping costs in a particular time step (USD per m^3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unit
; | ||
|
||
positive variables | ||
vm_watdem(wat_dem,j) Amount of water needed in different sectors (mio. m^3 per yr) | ||
v42_irrig_eff(j) Irrigation efficiency (1) | ||
vm_water_cost(i) Cost of irrigation water (USD per m^3 per yr) | ||
vm_water_cost(i) Cost of irrigation water (USD per m^3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -118,9 +118,9 @@ $include "./modules/42_water_demand/input/f42_env_flow_policy.cs3" | |||
$offdelim | |||
; | |||
|
|||
*Costs of pumping are taken from Cornish et.al. 2014 which are average global costs in USD | |||
*Costs of pumping are taken observed values in India and a conversion factor is applied to change them into USD values per cubic meter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refer to your study or add more detail on calculation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
parameter | ||
f42_pumping_cost(t_all,i) Cost of pumping irrigation water (1) | ||
f42_pumping_cost(t_all,i) Cost of pumping irrigation water (USD per m^3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are getting close to have this PR ready for approval. Most important step before that is to have a clear interface to switch off/on pumping costs in the model.
config/default.cfg
Outdated
cfg$gms$c41_initial_irrigation_area <- "LUH2v2" # def = LUH2v2 | ||
|
||
# * Sets the rate of depreciation of irrigation infrastructure in every timestep. | ||
# * Only applicable when (endo_apr13) realization is selected | ||
cfg$gms$s41_AEI_depreciation <- 0 # def = 0 | ||
|
||
|
||
# * switch for initialization area | ||
# * (LUH2v2): area equipped for irrigation based on LUH2v2 | ||
# * (Siebert): area equipped for irrigation from Siebert et al. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you move the description down here? Doesn't it belong to c41_initial_irrigation_area
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error. Fixed it.
@@ -6,4 +6,3 @@ | |||
# | Contact: magpie@pik-potsdam.de | |||
|
|||
name,type,reason | |||
vm_water_cost, variable, calculates cost of water pumping to be used in the costs module directly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please delete this file as there are no remaining entries in the table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from my side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s42_pumping = 0
does not work yet
if(m_year(t) > s42_multiplier_startyear, | ||
ic42_pumping_cost(i) = f42_pumping_cost(t,i)*s42_multiplier; | ||
); | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if s42_pumping
is set to 0, pumping costs will be active as you do set pumping costs and then use these pumping costs in the equations! You need to set ic42_pumping_cost(i) = 0;
for the case of s42_pumping = 0
!
*Pumping cost in the current time step | ||
if(m_year(t) > s42_pumping_startyear, | ||
ic42_pumping_cost(i) = f42_pumping_cost(t,i)*s42_multiplier; | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here as in the other realization: s42_pumping = 0
does not deactivate pumping costs
Felis points have been adressed now
🐦 Purpose of this PR 🐦
🔧 Checklist for PR creator 🔧
*Water Use at Global level - difference mainly due to India
![image](https://user-images.githubusercontent.com/51452385/173557083-c4eda2ea-aff3-491a-b7a0-3cbde6863258.png)
![image](https://user-images.githubusercontent.com/51452385/173557383-5ba3eb9b-43a7-48e4-9d7d-169cc96a9f00.png)
*Crop Production
![image](https://user-images.githubusercontent.com/51452385/173557495-49e9d54d-cb58-40d0-b98e-82e72b4e085b.png)
*Crop Production in India
![image](https://user-images.githubusercontent.com/51452385/173557580-e139181f-2afe-49c8-aea0-0b456842386d.png)
📉 Performance loss/gain from current default behavior 📈
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 (usegoxygen::goxygen()
for testing).Changes to
magpie4
R library for post processing of model output (ideally backward compatible).Self-review of my own code.
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%
NA
🚨 Checklist for RSE reviewer 🚨
CHANGELOG
is updated correctly🚨 Checklist for MAgPIE reviewer 🚨
CHANGELOG
is updated correctly