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

Add support for alpha_h1 parameter and the GLCNMO dataset #272

Merged
merged 11 commits into from
May 28, 2024

Conversation

JoostBuitink
Copy link
Collaborator

@JoostBuitink JoostBuitink commented May 17, 2024

Issue addressed

No issue, but fixes the last bullet in #226 (comment)

Also adds support for the GLCNMO land use dataset. This dataset is used for (global) models to identify regions where rice/paddy fields are located. Adding support for this dataset ensures more consistent model building.

Explanation

LULC parameters are based on existing tables. Alpha_h1 values are set to zero for all crops (not-natural vegetation), and set to 1 for all natural vegetation, based on the idea that natural vegetation is adapted to the local conditions and can deal with saturated conditions, while crops are likely not able to handle those conditions.

Checklist

  • Updated tests or added new tests
  • Branch is up to date with main
  • Tests & pre-commit hooks pass
  • Updated documentation if needed
  • Update default models (staticmaps) with new alpha_h1 layer
  • Update default models (toml) with new alpha_h1 layer

Additional Notes (optional)

Add any additional notes or information that may be helpful.

@JoostBuitink JoostBuitink marked this pull request as draft May 17, 2024 10:16
@JoostBuitink JoostBuitink marked this pull request as ready for review May 21, 2024 12:40
@alimeshgi alimeshgi requested a review from hboisgon May 23, 2024 07:31
Copy link
Contributor

@hboisgon hboisgon left a comment

Choose a reason for hiding this comment

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

Thanks for adding this @JoostBuitink !
Just very small comments before this is ready, I'll let you look into it.
Also nice that you added now the updating of the names in the toml file!

hydromt_wflow/data/parameters_data.yml Show resolved Hide resolved
hydromt_wflow/data/parameters_data.yml Outdated Show resolved Hide resolved
hydromt_wflow/wflow.py Outdated Show resolved Hide resolved
hydromt_wflow/wflow.py Outdated Show resolved Hide resolved
hydromt_wflow/wflow.py Show resolved Hide resolved
hydromt_wflow/wflow_sediment.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
Copy link
Contributor

@hboisgon hboisgon left a comment

Choose a reason for hiding this comment

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

Good now @JoostBuitink ! Just one small final comment but for the rest it's okay to merge!

hydromt_wflow/workflows/landuse.py Outdated Show resolved Hide resolved
@JoostBuitink JoostBuitink merged commit fc83571 into main May 28, 2024
5 of 6 checks passed
@JoostBuitink JoostBuitink deleted the lulc-alpha_h1-glcnmo branch May 28, 2024 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants