Added options for baseline protection #662

pvjeetze · 2024-04-12T15:48:37Z

🐦 Description of this PR 🐦

This PR adds options for choosing the baseline protection. This means that not only the full WDPA protection can be selected but also subsets of protected area categories (IUCN categories I, II & II and IUCN catergories IV, V & VI). In addition, no baseline protection at all can be chosen, which might be useful for certain test cases.
The default option of using all legally designated PAs in the historic period is not changed by this PR.

🔧 Checklist for PR creator 🔧

Label pull request from the label list.
- Low risk: Simple bugfixes (missing files, updated documentation, typos) or changes in start or output scripts
- Medium risk: Uncritical changes in the model core (e.g. moderate modifications in non-default realizations)
- High risk: Critical changes in model core or default settings (e.g. changing a model default or adjusting a core mechanic in the model)
Self-review own code
- No hard coded numbers and cluster/country/region names.
- The new code doesn't contain declared but unused parameters or variables.
- magpie4 R library has been updated accordingly and backwards compatible where necessary.
- scenario_config.csv has been updated accordingly (important if default.cfg has been updated)
Document changes
- Add changes to CHANGELOG.md
- Where relevant, put In-code documentation comments
- Properly address updates in interfaces in the module documentations
- run goxygen::goxygen() and verify the modified code is properly documented
Perform test runs
- Low risk:
  - Run a compilation check via Rscript start.R --> "compilation check"
- Medium risk:
  - Run test runs via Rscript start.R --> "test runs"
  - Check logs for errors/warnings
- High risk:
  - Run test runs via Rscript start.R --> "test runs"
  - Check logs for errors/warnings
  - Default run from the PR target branch for comparison
  - Provide relevant comparison plots (land-use, emissions, food prices, land-use intensity,...)

📉 Performance changes 📈

Current develop branch default : ** mins
This PR's default : ** mins

🚨 Checklist for reviewer 🚨

PR is labeled correctly
Code changes look reasonable
- No hard coded numbers and cluster/country/region names.
- No unnecessary increase in module interfaces
- model behavior/performance is satisfactory.
Changes are properly documented
- CHANGELOG is updated correctly
- Updates in interfaces have been properly addressed in the module documentations
- In-code documentation looks appropriate
content review done (at least 1)
RSE review done (at least 1)

…tion options

pvjeetze · 2024-04-23T09:48:33Z

Dear @ALL here's a brief reminder to please review this PR :)

…f_WDPABaselineSplit

flohump

Looks good. Thanks for adding the additional protection layers.
Please also update scenario_fsec.csv accordingly, i.e. input files and maybe protection layers.

pascal-sauer

Some clarifying questions, looks fine otherwise :)

pascal-sauer · 2024-04-24T07:22:32Z

scripts/output/extra/disaggregation.R

+    warning("No countries selected in land conservation disaggregation. Results may be erroneous")
+  }
+
+  if (!all(c(cfg$gms$c22_base_protect, cfg$gms$c22_base_protect_noselect) %in% "none")) {


Suggested change

if (!all(c(cfg$gms$c22_base_protect, cfg$gms$c22_base_protect_noselect) %in% "none")) {

if (cfg$gms$c22_base_protect != "none" || cfg$gms$c22_base_protect_noselect != "none") {

On second thought, this if statement does not actually do anything, does it? The same variables are checked individually afterwards.

This statementa checks is one of the two switches are not "none". If one is not "none" it moves into the bracket. Then in the bracket. The checks go into different procedures for the two switches. It could be that c22_base_protect is none, but c22_base_protect_noselect is switched to WDPA or vice versa. Therefore there are different checks later.

pascal-sauer · 2024-04-24T07:26:51Z

scripts/output/extra/disaggregation.R

+    }
+    if (base_protect_select != "none") {
+      land_consv_hr[consv_iso, , ] <- collapseDim(wdpa_hr[consv_iso, nyears(wdpa_hr), base_protect_select], dim = 3.1)
+    } else if (base_protect_select == "none") {


This if is redundant, isn't it?

Suggested change

} else if (base_protect_select == "none") {

} else {

This else would not be reached if the check in line 107 is FALSE, so that check actually can change things. Is that intended?

You are right. I just put it as well for better readability.

pascal-sauer · 2024-04-24T07:30:24Z

scripts/output/extra/disaggregation.R

-             message = "Write outputs peatland Mha")
+  comment = "unit: Mha per grid-cell",
+  message = "Write outputs peatland Mha"
+)


Did you use an auto-formatter? The way it was formatted before is compliant with the linter rules we have in the R packages, the new formatting is not.

lucode2::autoFormat() does not work with scripts. I used a different auto formatter.

weindl

Looks good.

weindl · 2024-04-24T11:08:48Z

modules/22_land_conservation/area_based_apr22/declarations.gms

- p22_country_dummy(iso)                                 Dummy parameter indicating whether country is affected by selected land conservation policy (1)
- i22_land_iso(iso)                          Total land area at ISO level (mio. ha)
+ p22_conservation_fader(t_all)                      Land conservation fader (1)
+ p22_add_consv(t,j,consv22_all,land)                Addtional land conservation in conservation priority areas (mio. ha)


small typo "Addtional"

…f_WDPABaselineSplit

pvjeetze · 2024-04-25T14:12:02Z

Looks good. Thanks for adding the additional protection layers. Please also update scenario_fsec.csv accordingly, i.e. input files and maybe protection layers.

@flohump I added the FSEC files. No changes are needed wrt to the used switches.

pvjeetze · 2024-05-14T16:42:39Z

@flohump I've addressed the comments, please see whether this PR can be approved and then merged.

flohump

Looks good.

pvjeetze added 2 commits April 12, 2024 17:36

added baseline conservation options

0ef652e

updated changelog

afe0354

pvjeetze requested review from flohump, alexkoberle and pascal-sauer April 12, 2024 15:48

pvjeetze added the Medium risk label Apr 12, 2024

pascal-sauer approved these changes Apr 15, 2024

View reviewed changes

pvjeetze added 5 commits April 15, 2024 15:03

modified extra/disaggregation to account for different basline protec…

ef4bc13

…tion options

new input data

91880a2

reverted run settings in main.gms

b1cdc4d

fix

bb9cafd

resolved merge conflicts

97aed0b

pvjeetze requested a review from weindl April 17, 2024 08:19

pascal-sauer self-requested a review April 18, 2024 07:28

Merge branch 'develop' of https://github.com/magpiemodel/magpie into …

6fea0a4

…f_WDPABaselineSplit

flohump requested changes Apr 23, 2024

View reviewed changes

pascal-sauer reviewed Apr 24, 2024

View reviewed changes

weindl approved these changes Apr 24, 2024

View reviewed changes

pvjeetze added 2 commits April 25, 2024 15:53

Merge branch 'develop' of https://github.com/magpiemodel/magpie into …

de05e08

…f_WDPABaselineSplit

PR review fixes

49473ac

pvjeetze requested review from flohump and pascal-sauer April 25, 2024 14:12

pascal-sauer approved these changes May 6, 2024

View reviewed changes

resolved merge conflicts

536be11

flohump approved these changes May 17, 2024

View reviewed changes

pvjeetze added 2 commits May 17, 2024 11:31

resolve merge conflict

79d13e1

bugfix

6f073dc

pvjeetze merged commit 5231701 into magpiemodel:develop May 17, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added options for baseline protection #662

Added options for baseline protection #662

pvjeetze commented Apr 12, 2024 •

edited

pvjeetze commented Apr 23, 2024

flohump left a comment

pascal-sauer left a comment

pascal-sauer Apr 24, 2024

pascal-sauer Apr 24, 2024

pvjeetze Apr 25, 2024

pascal-sauer Apr 24, 2024

pascal-sauer Apr 24, 2024

pvjeetze Apr 25, 2024

pascal-sauer Apr 24, 2024

pvjeetze Apr 25, 2024

weindl left a comment

weindl Apr 24, 2024

pvjeetze commented Apr 25, 2024

pvjeetze commented May 14, 2024

flohump left a comment

	if (!all(c(cfg$gms$c22_base_protect, cfg$gms$c22_base_protect_noselect) %in% "none")) {
	if (cfg$gms$c22_base_protect != "none" \|\| cfg$gms$c22_base_protect_noselect != "none") {

Added options for baseline protection #662

Added options for baseline protection #662

Conversation

pvjeetze commented Apr 12, 2024 • edited

🐦 Description of this PR 🐦

🔧 Checklist for PR creator 🔧

📉 Performance changes 📈

🚨 Checklist for reviewer 🚨

pvjeetze commented Apr 23, 2024

flohump left a comment

Choose a reason for hiding this comment

pascal-sauer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weindl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvjeetze commented Apr 25, 2024

pvjeetze commented May 14, 2024

flohump left a comment

Choose a reason for hiding this comment

pvjeetze commented Apr 12, 2024 •

edited