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

Feat/echam streams #1240

Open
wants to merge 15 commits into
base: release
Choose a base branch
from
Open

Feat/echam streams #1240

wants to merge 15 commits into from

Conversation

pgierz
Copy link
Member

@pgierz pgierz commented Oct 24, 2024

Automatically create ECHAM streams from mvstreamctl filetags.

Should be useful for #1239

@pgierz pgierz marked this pull request as ready for review October 24, 2024 14:08
Copy link
Contributor

@mandresm mandresm left a comment

Choose a reason for hiding this comment

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

Nice work @pgierz :) Hopefully will make many echam headaches go away.

Just a few comments

src/esm_runscripts/echam.py Outdated Show resolved Hide resolved
src/esm_runscripts/echam.py Outdated Show resolved Hide resolved
namelist = f90nml.read(econfig["namelist_dir"] + "/namelist.echam")
mvstream_tags = _get_mvstream_tags_from_namelist(namelist)
jsbach_streams = config["jsbach"]["streams"]
ignore_these_tags = econfig.get("ignore_tags", jsbach_streams)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the default ignore ignore jsbach_streams? Is it so that the jsbach streams are moved to the jsbach directory instead of echam?

I find this behavior a bit confusing because if I declare an echam.ignore_tags then the jsbach streams won't be ignored anymore, so there is a big change of behavior behind the scenes. What do you think @pgierz? Maybe I am not understanding this part...

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, see new commit (coming later today)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in d6aab9.

return mvstream_tags


def append_namelist_dependent_sources(config):
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this happen for echam but also jsbach? Doesn't jsbach also have its own streams?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a little bit convoluted: jsbach and echam are controlled by the same namelist, so I would need some way to filter out which tags go to which model. In principle this would just be defined by which model config they come from.

Copy link
Member Author

Choose a reason for hiding this comment

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

See 509ac21.

Copy link
Contributor

Choose a reason for hiding this comment

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

jsbach and echam are controlled by the same namelist

Do you mean that they can be controlled from the same namelist? If that is the case, then I understand the rest of your reply and the new jsbach.py

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the output generated from jsbach is defined in mvstreamctl chapters of namelist.echam (since JSBACH technically speaking is just a submodule inside of ECHAM), and does not have any output control in it's own namelist.

@pgierz pgierz added the jsbach label Nov 4, 2024
@pgierz pgierz requested a review from mandresm November 4, 2024 09:36
@pgierz
Copy link
Member Author

pgierz commented Nov 4, 2024

The latest few commits use the extremely useful feature of being able to write tests directly in your docstrings under the examples section. The included files are referenced directly from the root of the repo in the CI actions. Check it out 😄

src/esm_runscripts/echam.py::esm_runscripts.echam._get_mvstream_tags_from_namelist PASSED [ 71%]
src/esm_runscripts/jsbach.py::esm_runscripts.jsbach._get_comments_for_streams PASSED [ 85%]

@mandresm
Copy link
Contributor

mandresm commented Nov 4, 2024

Some of the dryruns in esm_tests are failing

src/esm_runscripts/echam.py Outdated Show resolved Hide resolved
return mvstream_tags


def append_namelist_dependent_sources(config):
Copy link
Contributor

Choose a reason for hiding this comment

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

jsbach and echam are controlled by the same namelist

Do you mean that they can be controlled from the same namelist? If that is the case, then I understand the rest of your reply and the new jsbach.py

@mandresm
Copy link
Contributor

mandresm commented Nov 4, 2024

Once the CI tests work, I would recommend running esm_tests in levante with this branch to check that also actual simulations run correctly. I can take care of that to unload you a bit @pgierz. Just let me know when I can go ahead and run the real tests

@pgierz
Copy link
Member Author

pgierz commented Nov 4, 2024

Should be good to go. The failing levante tests mainly fail due to changes in how the streams get picked up, as well as new main keys in the echam config, e.g.:

  	Differences in run/awicm/awicm1-CMIP6-initial-monthly//run_18500101-18500131/config//awicm1-CMIP6-initial-monthly_finished_config.yaml:
  		--- 
  		+++ 
  		@@ -118,6 +118,7 @@
  		   forcing_dir: /pool/data/ECHAM6//input/r0007/T127
  		   greenhouse_dir: /pool/data/ECHAM6/
  		   namelist_dir: <HOME_DIR>/esm_tools/namelists//echam/6.3.04p1/PI-CTRL
  		+  has_namelist_streams: true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants