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

Improved the _ravenpy_models.py script to be able to specify the rain snow fraction for all models #215

Merged
merged 3 commits into from
Oct 16, 2024

Conversation

lou-a
Copy link
Contributor

@lou-a lou-a commented Oct 15, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes #xyz
  • (If applicable) Documentation has been added / updated (for bug fixes / features).
  • (If applicable) Tests have been added.
  • CHANGELOG.rst has been updated (with summary of main changes).
    • Link to issue (:issue:number) and pull request (:pull:number) has been added.

What kind of change does this PR introduce?

Improved the _ravenpy_models.py script to be able to specify the rain snow fraction for all models, including HBVEC. With the new version of RavenPy (v0.16.0, unreleased), the rain_snow_fraction could be set with "RainSnowFraction=rain_snow_fraction" in the self.default_emulator_config dictionary. See: CSHS-CWRA/RavenPy#408. For now (based on the latest release RavenPy version, v0.15.0) the change I'm suggesting here will enable changing the rain snow fraction algorithm for HBVEC as well.

Does this PR introduce a breaking change?

None foreseen.

Other information:

Copy link

github-actions bot commented Oct 15, 2024

Welcome, new contributor!

It appears that this is your first Pull Request. To give credit where it's due, we ask that you add your information to the AUTHORS.rst and .zenodo.json:

  • The relevant author information has been added to AUTHORS.rst and .zenodo.json.

Please make sure you've read our contributing guide. We look forward to reviewing your Pull Request shortly ✨

Copy link
Contributor

@richardarsenault richardarsenault left a comment

Choose a reason for hiding this comment

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

Looks good. Did not know that this command structure (rain_snow_fraction vs RainSnowFraction keyword) worked too, but it seems the tests are passing.

@github-actions github-actions bot added the approved Approved for additional tests label Oct 16, 2024
@lou-a
Copy link
Contributor Author

lou-a commented Oct 16, 2024

Looks good. Did not know that this command structure (rain_snow_fraction vs RainSnowFraction keyword) worked too, but it seems the tests are passing.

Thank you for checking, Richard. You can use either terms for most emulators, depending on the aliases they use for the processes. Same for other processes, as far as I know. For this specific example, the rain snow fraction process is defined like so in most emulators: o.RainSnowFraction = Field("RAINSNOW_HBV", alias="RainSnowFraction"), where the alias is key to being able to call RainSnowFraction. In HBVEC this was defined without any alias (fixed in RavenPy v0.16.0): o.RainSnowFraction = Field("RAINSNOW_HBV").

@lou-a lou-a merged commit c0186be into main Oct 16, 2024
24 checks passed
@lou-a lou-a deleted the ravenpy_models_improvement branch October 16, 2024 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants