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

run all output scripts on one slurm, improve logging #1340

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

orichters
Copy link
Contributor

@orichters orichters commented Jun 19, 2023

Purpose of this PR

  • add --vanilla to Rscript calls within GAMS to suppress hundreds of Global .Rprofile loaded! (R version 4.1.2 (2021-11-01)) lines in the logs.
  • In output.R, if you select single but then choose more than one output script (say: reporting + rds_report + emulator for a coupled run), they were all run on separate slurm jobs, sometimes interfering with each other, or one script needed the output of the other, which creates a big mess. I propose to run each outputdir in one slurm job, but all the scripts selected one after the other in this one job. This allows you to actually select a chain of output scripts that need each other, instead of having them interfere with each other.
  • in scripts/input/exoGAINSAirpollutants.R, improve the logging which file is used.
  • in start.R, also look into config if you find the config file there. I struggled quite a bit with the warning that the file could not be found, although it was clearly in the config folder, except I forgot to add the config/ part in the command line. Make sure this does not happen anymore to anyone.

Type of change

  • Bug fix
  • Refactoring

Checklist:

  • My code follows the coding etiquette
  • I performed a self-review of my own code
  • I explained my changes within the PR, particularly in hard-to-understand areas
  • I checked that the in-code documentation is up-to-date
  • I did not adjust the reporting in remind2
  • All automated model tests pass (FAIL 0 in the output of make test)

Copy link
Member

@LaviniaBaumstark LaviniaBaumstark left a comment

Choose a reason for hiding this comment

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

thanks for all the cleaning

message("\nFinished ", ifelse(slurmConfig == "direct", "", "starting job for "), "output generation for ", outputdir, "!\n")
} else {
warning("Skipping ", outputdir, " because some output script selected could not be found ",
"in scripts/output/single: ", name[! name %in% dir("scripts/output/single")])
Copy link
Member

Choose a reason for hiding this comment

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

I like your proposal to run more than one R-scripts in one slurm job to avoid mess

@orichters orichters merged commit c623583 into remindmodel:develop Jun 21, 2023
@@ -19,7 +19,7 @@ if((o_modelstat le 2),
);

*** Calculate AP emissions
Execute "Rscript exoGAINSAirpollutants.R";
Execute "Rscript --vanilla exoGAINSAirpollutants.R";

Choose a reason for hiding this comment

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

I'll state to obvious (at least obvious to anyone who read the R man page):
this deactivates the renv, making consistent results from REMIND runs a gamble once the versions of the packages required by ecoGAINSAirpollutants.R diverge between the renv library and whatever happens to be in R_LIBS_SITE.

@orichters orichters mentioned this pull request Jun 21, 2023
6 tasks
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