Ravenpy models #128
Ravenpy models #128
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
xhydro/modelling/_ravenpy_models.py
Outdated
| # Create HRU object for ravenpy based on catchment properties | ||
| self.model_simulations = None | ||
| self.qsim = None | ||
| hru = dict( | ||
| area=drainage_area, | ||
| elevation=elevation, | ||
| latitude=latitude, | ||
| longitude=longitude, | ||
| hru_type="land", | ||
| ) |
There was a problem hiding this comment.
On va probablement aussi vouloir qu'un utilisateur puisse pointer vers un fichier de HRU déjà construit? Je pense ici au travail pour l'Atlas, par exemple.
There was a problem hiding this comment.
Hmmm c'est un bon point. En fait, ce qu'on voudrait, c'est qu'un usager puisse prendre ses fichiers .rvX au complet et rouler dans xhydro comme n'importe quel modèle. On a des outils pour ça aussi. Laisse-moi voir ce que je peux faire.
There was a problem hiding this comment.
Bon, à bien y penser, je ne suis pas certain que ça soit idéal d'avoir les modèles custom dans xhydro (du moins pour Ravenpy). Ravenpy a déjà un mécanisme permettant de "loader" un modèle à partir de sa configuration déjà construite et de le faire tourner. Le code pour le faire est très simple:
from ravenpy import OutputReader
from ravenpy.ravenpy import run
# Get the .rvX files paths
config = [
get_file(f"raven-gr4j-cemaneige/raven-gr4j-salmon.{ext}")
for ext in ["rvt", "rvc", "rvi", "rvh", "rvp"]
]
configdir = config[0].parent
outputs_path = run(modelname="raven-gr4j-salmon", configdir=configdir, overwrite=True)
outputs = OutputReader(run_name=None, path=outputs_path)
Cependant, on ne peut pas calibrer ces modèles car ils peuvent avoir toutes sortes de paramètres custom. Si on fait un modèle spécifique pour le projet (ex. ravenpy HBVEC distribué) alors je crois que c'est mieux d'avoir une fonction spécifique pour celui-là.
Le HRU tel qu'il est indiqué ici est la manière de faire pour les modèles qu'on a "en stock" dans ravenpy actuellement alors je ne le changerais pas.
Qu'en penses-tu?
There was a problem hiding this comment.
Est-ce que l'idée serait que l'utilisateur puisse faire tourner le modèle 1) à partir de ses fichiers .rvx ou 2) d'une shapefile des HRUs avec un NetCDF des inputs météo ? Dans le cas 1) on n'aurait même pas besoin de créer la configuration de l'émulateur ("# Create the emulator configuration") si je ne me trompe pas ? Si on préfère aller vers l'option 2), je suggérerais d'aller vers quelque chose d'un peu plus flexible pour créer le .rvh de HRUs et de remplacer cette partie du code (lignes 95-101) avec :
HRUs_gdf = gpd.read_file(hru_file.shp)
rvh_config = BasinMakerExtractor(HRUs_gdf).extract()
Puis ensuite lorsque l'on crée l'émulateur (ligne 104) on peut fournir au dictionnaire :
sub_basins=rvh_config['sub_basins'],
sub_basin_group=rvh_config['sub_basin_group'], # optionnel ?
reservoirs=rvh_config['reservoirs'], # optionnel
channel_profile=rvh_config['channel_profile'],
hrus=rvh_config['hrus'],
Qu'en pensez vous ?
| @@ -22,6 +22,7 @@ dependencies: | |||
| - spotpy | |||
| - stackstac | |||
| - statsmodels | |||
| - ravenpy | |||
There was a problem hiding this comment.
Pour ce fichier (et les autres associés), tu pourrais quand même mettre une version minimal si tu penses que c'est nécessaire pour ce que tu as fait.
|
@Zeitsperre Hi! I see there are dependencies failing and some tests that are not related to this PR that are also failing. Any idea what these are / how to fix (especially the dependencies, which I suspect are playing a role in the failing unit tests?) |
|
We made some breaking changes to a few statistical functions in Edit: The changes don't appear to be coming from |
|
Just to not block this PR, we could temporarily add a pin to both xclim ( |
Pull Request Checklist:
number) and pull request (:pull:number) has been added.What kind of change does this PR introduce?
This PR implements classes for ravenpy modelling and model calibration, instead of dictionary and function-based modelling.
Does this PR introduce a breaking change?
No, but one test for hydrotel fails that is unrelated (I think).
Other information: