search

esm-tools / esm_tools

Simple Infrastructure for Earth System Simulations
https://esm-tools.github.io/
GNU General Public License v2.0
25 stars 12 forks source link

Bugfix for setting use_venv in runscript having no effect. #1151

Closed nwieters closed 5 months ago

nwieters commented 6 months ago

Closes #1150

nwieters commented 6 months ago

Need to do more testing since --contained-run is now/still breaking.

nwieters commented 5 months ago

I reactivated the lines that take care of the use_venv setting from the runscript.

nwieters commented 5 months ago

Hi, I rerun the failing tests and they are now passing. Please feel free to review and also merge this into release.

nwieters commented 5 months ago

Hi @mandresm , as I mentioned already, these changes let the setting in the runscript always win or give an error if runscript setting and argument are not campatible. You may want to change this behaviour. When I started to fix this bug I had a different approach. Maybe you can have a look at that again, if this is useful for you. It is in the commit a269640a0292127e4193467958c7511fc3b26606

pgierz commented 5 months ago

Just so I understand this correctly: my current mental model of how this works is as follows:

Is that more or less correct? Or did I misunderstand the overwrite order?

mandresm commented 5 months ago

Just so I understand this correctly: my current mental model of how this works is as follows:

* Command line arguments win, always. They are recycled run to run (e.g. the next run resubmits itself with the same flags as were given the first time)

* User specifications in the runscript are next. Changing the submission script between runs (assuming it is stored in a different location) has no influence on the run, since it will recycle the on in `${EXP_BASE}/scripts` and not `/some/user/location/scripts/my_run.yaml`

* Changing `${EXP_BASE/scripts/my_run.yaml` **will** change what happens, but if _and only if_ the options are not overridden by a command line flag

* Internal (e.g. component/setup/etc) configuration files come last.

Is that more or less correct? Or did I misunderstand the overwrite order?

That is correct. Somehow we had it the other way around only for use_venv, and I don't remember why, but to me this is nonsense (even if I implemented that approach myself) so I'm bringing things back to "normal"

mandresm commented 5 months ago

All tests passing so... #bump #patch

pgierz commented 5 months ago

Just so I understand this correctly: my current mental model of how this works is as follows:

* Command line arguments win, always. They are recycled run to run (e.g. the next run resubmits itself with the same flags as were given the first time)

* User specifications in the runscript are next. Changing the submission script between runs (assuming it is stored in a different location) has no influence on the run, since it will recycle the on in `${EXP_BASE}/scripts` and not `/some/user/location/scripts/my_run.yaml`

* Changing `${EXP_BASE/scripts/my_run.yaml` **will** change what happens, but if _and only if_ the options are not overridden by a command line flag

* Internal (e.g. component/setup/etc) configuration files come last.

Is that more or less correct? Or did I misunderstand the overwrite order?

That is correct. Somehow we had it the other way around only for use_venv, and I don't remember why, but to me this is nonsense (even if I implemented that approach myself) so I'm bringing things back to "normal"

There was probably a good reason at the time, but I can't remember it either ;-) This is cleaner anyway. So, all green!