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

esmvaltool update breaks Marrmot forcing #322

Closed
RolfHut opened this issue Nov 1, 2022 · 3 comments · Fixed by #323
Closed

esmvaltool update breaks Marrmot forcing #322

RolfHut opened this issue Nov 1, 2022 · 3 comments · Fixed by #323
Labels
bug Something isn't working

Comments

@RolfHut
Copy link
Contributor

RolfHut commented Nov 1, 2022

I think an update in esmvaltool, or some other update somewhere, has broken the marrmot forcing routine in eWaterCycle.

line 73 in https://github.com/eWaterCycle/ewatercycle/blob/main/src/ewatercycle/forcing/_marrmot.py assumes that the first element in recipe_output contains the final resulting .mat file, but it does not anymore: now it points to the preproc directory.

I think line 73 needs to point more explicitly to the .mat file, without relying on the order of recipe_output to remain the constant?

See attached screenshot of the output:

Screenshot 2022-11-01 at 10 52 11

@RolfHut RolfHut added the bug Something isn't working label Nov 1, 2022
@Peter9192
Copy link
Collaborator

This is where ewatercycle picks up the forcing file:

# generate forcing data and retrieve useful information
recipe_output = recipe.run(session=_session(directory))
forcing_file: Path = list(recipe_output.values())[0].files[0].path

That index-based lookup seems a bit fragile indeed

@Peter9192
Copy link
Collaborator

Peter9192 commented Nov 1, 2022

Since ESMValGroup/ESMValCore@49c65db, preproc files are also included in the output retreived from the esmvaltool python api, unless the setting remove_preproc_dir is True.

@Peter9192
Copy link
Collaborator

It would be good to add a check that we indeed get a .mat file in ewatercyce/forcing/_marrmot.py

RolfHut added a commit that referenced this issue Nov 1, 2022
RolfHut added a commit that referenced this issue Nov 1, 2022
Peter9192 added a commit that referenced this issue Nov 29, 2022
* fix for #322 marrmot forcing

* Fix linter issues

* add tests - not working yet

* fix tests

* fix linter issues

* inline comment

* Update expected test outputs after to esmvaltool update

* Update CHANGELOG.md

Co-authored-by: Peter Kalverla <peter.kalverla@gmx.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants