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

eWaterCycle: Add recipe to prepare input for HYPE #1334

Merged
merged 34 commits into from
Feb 20, 2020

Conversation

ipelupessy
Copy link
Contributor

@ipelupessy ipelupessy commented Sep 25, 2019

Short description of the diagnostic
Convert ERA-Interim and/or ERA5 meteorological input data to a format that can be used as forcing data for the HYPE hydrological model. This is part of the preprocessing workflow in the eWaterCycle project.


Tasks

  • Create an issue to discuss what you are going to do, if you haven't done so already (and add the link at the bottom)
  • Make sure your code is composed of functions of no more than 50 lines and uses meaningful names for variables
  • Circle/CI tests pass. Status can be seen below your pull request. If the tests are failing, click the link to find out why.
  • Preferably Codacy code quality checks pass, however a few remaining hard to solve Codacy issues are still acceptable. Status can be seen below your pull request. If there is an error, click the link to find out why. If you suspect Codacy may be wrong, please ask by commenting.
  • Please use yamllint to check that your YAML files do not contain mistakes
  • n/a (Only if really necessary) Add any additional dependencies needed for the diagnostic script to setup.py, esmvaltool/install/R/r_requirements.txt or esmvaltool/install/Julia/julia_requirements.txt (depending on the language of your script) and also to meta.yaml for conda dependencies (includes Python and others, but not R/Julia)
  • n/a If new dependencies are introduced, check that the license is compatible with Apache2.0

New recipe/diagnostic

  • Add documentation for the recipe to the doc/sphinx/source/recipes folder and add a new entry to index.rst

Closes #1335

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

one change and one question. Cheers for adding this! 🍺

esmvaltool/diag_scripts/hydrology/hype.py Show resolved Hide resolved
esmvaltool/recipes/hydrology/recipe_hype.yml Outdated Show resolved Hide resolved
@bouweandela
Copy link
Member

Could you please merge the version2_development branch into this branch, so the unit tests on CircleCI work?

@JaroCamphuijsen JaroCamphuijsen changed the title Hype input generator eWaterCycle: Add recipe to prepare input for HYPE Oct 2, 2019
@ipelupessy
Copy link
Contributor Author

circleci fails, but on existing (style) issues...

@ipelupessy
Copy link
Contributor Author

ok, it still needs a few changes in the diag_script with respect to formatting and units..

@Peter9192
Copy link
Contributor

@ipelupessy could you add support for the ERA5 dataset as well (see https://github.com/ESMValGroup/ESMValTool/blob/pcrglobwb_era5_recipes/esmvaltool/recipes/hydrology/recipe_pcrglobwb.yml for an example). Basically, it boils down to this:

  • Add preprecessors for hourly data (duplicate yours and add daily_statistics)
  • Add a diagnostic for hourly data (duplicate yours and use the additional_datasets keyword within each of the diagnostics, rather than the top level datasets specification.

@ipelupessy
Copy link
Contributor Author

@Peter9192 you zapped me from the contributors list, this breaks the recipe ;-)

@Peter9192
Copy link
Contributor

Sorry 'bout that @ipelupessy , I was under the impression that you would already be listed in my copy of the development version. It's quite annoying that there are so many versions of this file in various branches. Actually, there is an issue about this: ESMValGroup/ESMValCore#28

Note that this possibly undoes some bonafide changes that have slipped
in; this has the potential to undo the commit in a later merge. These
should be carefully done
Co-Authored-By: Bouwe Andela <bouweandela@users.noreply.github.com>
@mattiarighi mattiarighi changed the base branch from version2_development to master January 3, 2020 17:57
@ipelupessy
Copy link
Contributor Author

@bouweandela, @valeriupredoi as far as i can see this is good to merge (but maybe I am overlooking something...)

@bouweandela bouweandela merged commit b383f94 into ESMValGroup:master Feb 20, 2020
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.

eWaterCycle: Add recipe to prepare input for HYPE
6 participants