-
Notifications
You must be signed in to change notification settings - Fork 173
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
Start script 'healthy landscapes paper' and output script for automatic submission of SEALS runs #721
Conversation
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. Thanks for these improvements.
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, thank you!
@@ -737,7 +737,7 @@ cfg$gms$s29_snv_scenario_target <- 2050 # def = 2050 | |||
# * plausible options: "secdforest, forestry, past, other", | |||
# * "secdforest, other", | |||
# * "secdforest, past, other" etc. | |||
cfg$gms$land_snv <- "secdforest, forestry, past, other" #def = "secdforest, forestry, past, other" | |||
cfg$gms$land_snv <- "secdforest, other" #def = "secdforest, other" |
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.
Maybe add to the changelog that the default setting for the SVN policy has changed.
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 updated the changelog. Thanks for spotting this
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 to me, just need to deal with the paths to your personal folders
|
||
### Path to miniforge installation | ||
# For instructions on how to install miniforge see https://github.com/conda-forge/miniforge | ||
miniforgePath <- "/home/vjeetze/miniforge3/bin/activate" |
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.
Paths into home directories are not ideal, I get permission denied, and even if you give permission this only works as long as your home folder exists.
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.
How do you want this script to be used by others, should they install miniforge themselves (into their home folder) and then modify the script here? Or can we setup a shared miniforge path in /p/projects/rd3mod
maybe?
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 goes back to the whole discussion whether miniforge should be available to all (e.g. via the piam module or else). At the moment I could only install it into my home directory. This may be fine for the time being and if any users want to use the script they can install it in their home directory and change the path.
I don't expect this script to be used by anyone besides me at the moment. I want this to be part of the release in the current form mainly for reasons of documentation for a paper. However, the script is also fully functional, if the user settings are modified based on user specific installations of miniforge and SEALS.
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've adapted the script based on our bilateral discussion. Please have a look whether you are fine with these changes.
Co-authored-by: Pascal Sauer <[email protected]>
Co-authored-by: Pascal Sauer <[email protected]>
Co-authored-by: Pascal Sauer <[email protected]>
Co-authored-by: Pascal Sauer <[email protected]>
🐦 Description of this PR 🐦
.nc
files are no longer exported by default after disaggregation of MAgPIE outputsmagpie4
dependency to2.12.0
to adjust lowpass filter based on sm_fix_SSP2 in emisCO2🔧 Checklist for PR creator 🔧
Label pull request from the label list.
Self-review own code
magpie4
R library has been updated accordingly and backwards compatible where necessary.scenario_config.csv
has been updated accordingly (important ifdefault.cfg
has been updated)Document changes
CHANGELOG.md
goxygen::goxygen()
and verify the modified code is properly documentedPerform test runs
Rscript start.R --> "compilation check"
Rscript start.R --> "test runs"
Rscript start.R --> "test runs"
📉 Performance changes 📈
🚨 Checklist for reviewer 🚨
CHANGELOG
is updated correctly