Skip to content

Refactor output.R#2413

Open
simonkrogmann wants to merge 1 commit into
remindmodel:developfrom
simonkrogmann:output-cleanup
Open

Refactor output.R#2413
simonkrogmann wants to merge 1 commit into
remindmodel:developfrom
simonkrogmann:output-cleanup

Conversation

@simonkrogmann

@simonkrogmann simonkrogmann commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Purpose of this PR

  • I replaced parts of the command line parsing with the standard solution optparse (already a dependency of remind) instead of a self-rolled script in remind. This avoids dumping all parameters into the global namespace which is error-prone. The help function and documentation is now more like to stay up-to-date.
  • I split the script into functions and prepared a later refactoring where the whole script can be called as a normal function.
  • I removed some backwards compatibility code that seem to not have been used for years. Is it ok to remove them.
    • supporting ,cfg instead of .Rdata run files
    • boolean flag comp

I have tested some scripts with some parameter configurations but an exhaustive test is infeasible for me.

Type of change

Indicate the items relevant for your PR by replacing ◻️ with ☑️.
Do not delete any lines. This makes it easier to understand which areas are affected by your changes and which are not.

Parts concerned

  • ◻️ GAMS Code
  • ☑️ R-scripts
  • ☑️ Documentation (GAMS incode documentation, comments, tutorials)
  • ◻️ Input data / CES parameters
  • ◻️ Tests, CI/CD (continuous integration/deployment)
  • ◻️ Configuration (switches in main.gms, default.cfg, and scenario_config*.csv files)
  • ◻️ Other (please give a description)

Impact

  • ◻️ Bug fix
  • ☑️ Refactoring
  • ◻️ New feature
  • ◻️ Change of parameter values or input data (including CES parameters)
  • ◻️ Minor change (default scenarios show only small differences)
  • ◻️ Fundamental change of results of default scenarios

Checklist

Do not delete any line. Leave unfinished elements unchecked so others know how far along you are.
In the end all checkboxes must be ticked before you can merge
.

  • I executed the automated model tests (make test) after my final commit and all tests pass (FAIL 0)
  • I adjusted the reporting in remind2 if and where it was needed
  • I adjusted the madrat packages (mrremind and other packages involved) for input data generation if and where it was needed
  • My code follows the coding etiquette
  • 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 adjusted forbiddenColumnNames in readCheckScenarioConfig.R in case the PR leads to deprecated switches
  • I updated the CHANGELOG.md correctly (added, changed, fixed, removed, input data/calibration)

Further information (optional)

I'm happy about feedback on the renv handling that this PR touches only tangentially.

@simonkrogmann simonkrogmann requested a review from fbenke-pik July 16, 2026 09:19
Comment thread output.R

# run updates before loading any packages
if ("--update" %in% argv) {
stopifnot(`--update must not be used together with --renv=...` = !any(startsWith(argv, "--renv=")))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This could be simplified further if loading optparse before the renv is a possible. I;m not sure whether this is possible.

Comment thread scripts/start/run.R
Comment thread output.R
gms$scenarios <- strsplit(grep("(cfg\\$|)gms\\$scenarios +<-", l, value = TRUE), "\"")[[1]][2]
revision <- as.numeric(unlist(strsplit(grep("(cfg\\$|)inputRevision +<-", l, value = TRUE), "<-[ \t]*"))[2])
}
# Old .cfg files will not be read anymore

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you get confirmation from someone in RSE that .cfg files are indeed not used anymore?

@simonkrogmann simonkrogmann Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@dklein-pik Do you know anything about the .cfg format? To me it looked deprecated since at least 5 years. They were replaced with config.Rdata

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, the section can be removed. Thanks.

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