fix cm_repeatNonOpt regexp

JakobBD commented 3 months ago

Also change non-'off' value from 'yes' to more intuitive 'on' (has no effect, since is only compared to 'off' string in code)

Purpose of this PR

bugfix: The lack of regexp throws an error when running REMIND with scenario_config.csv, as it misses the cm_repeatNonOpt switch in the config file.

Type of change

(Make sure to delete from the Type-of-change list the items not relevant to your PR)

[x] Bug fix

Checklist:

[x] My code follows the coding etiquette
[x] I performed a self-review of my own code
[ ] I explained my changes within the PR, particularly in hard-to-understand areas
[x] I checked that the in-code documentation is up-to-date
[x] I adjusted the reporting in remind2 where it was needed
[x] I adjusted forbiddenColumnNames in readCheckScenarioConfig.R in case the PR leads to deprecated switches
[ ] All automated model tests pass (FAIL 0 in the output of make test)
[x] The changelog CHANGELOG.md has been updated correctly

Further information (optional):

Test runs are here:
Comparison of results (what changes by this PR?):

JakobBD commented 3 months ago

Added a bit of context to the description. Comments, anyone? It's a very small change, but necessary for REMIND to run. I'm a bit surprised no-one else seems to have trouble with this. I'm merging the fix manually into all my branches.

orichters commented 3 months ago

Did REMIND really crash for you without the regexp? This should really not be the case! Can you provide an example where that happens, @JakobBD?

JakobBD commented 3 months ago

/p/tmp/jakobdu/remind_pbs/output/SSP2EU-NPi_2024-03-20_17.17.02/log.txt I changed the code in the repo since then, but the log file lists the commit hash, if you'd like to reproduce the error.

orichters commented 3 months ago

Honestly, that is a complete mystery for me. Are you sure you did not adjust the main.gms somewhat between the creation of the rdata file and the moment the model run was prepared? Because if I check out the commit hash and run gms::readDefaultConfig(".")$gms$cm_repeatNonOpt returns "off" as it should, while if you load config.Rdata, this switch is indeed missing in cfg$gms. But basically, the cfg is read twice using the same code as far as I see, and I really wonder how this might have happened that the switch is present only once without any change in main.gms.

Might it be that between you send the run to slurm and the Rdata file was written, and the run actually started, you pulled from develop, caught this commit and therefore the switch was missing in your cfg? Just a guess, but I think that is more probable than the code being wrong that works for everyone else.

Strong support for this theory: SSP2EU-NPi_2024-03-20_17.17.02 states the run was started at 17.17, while your latest commit according to the logfile is 2024-03-20 17:22:55. So I think that is no mystery anymore :)

JakobBD commented 3 months ago

Ah, interesting. Then I just got misled by the Either adapt the parameter choice or the regexp in main.gms error message.

remindmodel / remind