Open tyralla opened 7 months ago
- Rename grxj_land. I propose to rename the model group grxjland to gr_land (gr for genie rurale).
This suggestion is taken from the PR. There seems to be no fundamental difference between the model versions for daily and hourly simulation steps. Hence, committing the "j" is reasonable. Then, the "x" is not required as a placeholder anymore. However, we only took the first letters from HBV96, LARSIM, and WALRUS (hland
, lland
, and wland
) to make clearer distinctions between the original and the HydPy implementations. So, the alternative suggestion is gland
. However, note that a suggested name for one of the evap models is currently evap_pet_hbv96
. What do others think?
I would prefer gr_land as model group name.
-----Ursprüngliche Nachricht----- Von: Christoph Tyralla @.> Gesendet: Dienstag, 16. April 2024 09:39 An: hydpy-dev/hydpy @.> Cc: Klein, Bastian, M2, B40 @.>; Mention @.> Betreff: Re: [hydpy-dev/hydpy] Finish the implementation of GR4J and friends (Issue #134)
* Rename grxj_land. I propose to rename the model group grxjland to gr_land (gr for genie rurale).
This suggestion is taken from the PR https://github.com/hydpy-dev/hydpy/pull/112#issue-1678441435 . There seems to be no fundamental difference between the model versions for daily and hourly simulation steps. Hence, committing the "j" is reasonable. Then, the "x" is not required as a placeholder anymore. However, we only took the first letters from HBV96, LARSIM, and WALRUS (hland, lland, and wland) to make clearer distinctions between the original and the HydPy implementations. So, the alternative suggestion is gland. However, note that a suggested name for one of the evap models is currently evap_pet_hbv96. What do others think?
— Reply to this email directly, view it on GitHub https://github.com/hydpy-dev/hydpy/issues/134#issuecomment-2058438285 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AOZHFRAEQ55RNJ7DO7EGKLLY5TISXAVCNFSM6AAAAABGIY2EBKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJYGQZTQMRYGU . You are receiving this because you were mentioned. https://github.com/notifications/beacon/AOZHFRA7K26YJBMCCGOUNTDY5TISXA5CNFSM6AAAAABGIY2EBKWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTT2WFDI2.gif Message ID: @.***>
@sivogel also raised the question (in an offline discussion) of whether to extract the unit hydrograph into a submodel (see #130). Currently, @parvumflumen implements a suitable submodel family in #133. I see no reason against this (except about one or two days of additional work).
Please note the related discussion in #130.
I would prefer gr_land as model group name.
@sivogel: When (or if) you apply this name, could you please carefully check if the underscore does not cause any problems? Code sections might assume an application model's family name to be the first substring before the underscore. I would most likely expect such assumptions in the code that generates the documentation.
The current proposal suggests three application models with limited differences. Can we reduce the number of application models without losing functionality? This could help remove some redundancies (especially in testing). However, we should refrain from such refactoring if it affects comprehensibility (too much).
I would prefer gr_land as model group name.
I remembered one place where this name could cause problems: autodoc_applicationmodel (see the split operation; I guess we explain the "conventional way" somewhere else)
In an offline discussion, we decided on gland
.
Currently we have 3 different gland application models gland_gr4, gland_gr5 and gland_gr6. @tyralla asked if it is possible to merge these 3 application models into one application model. In my opinion we should stay with 3 application models because I do not see a way to merge Calc_F_V1 and Calc_F_V2 into 1 application model without having meaningless parameters.
But maybe we can combine the snow model with hysteresis effect with the one without. To do this, we would need to introduce GLocalMax into the model without hysteresis effect. This would be an overhead in the simpler application model, but would save us two application models.
The method Calc_PLayer_V1 calculates the precipitation for each height level of the snow module. In airGR it is assumed that above 4000m precipitation is no longer height dependent. This assumption was not found in any other publication. Should we keep the airGR assumption?
I could not find any differences between the hourly and the daily gr4j airGR model. The Fortran scripts on airGR differ only in the parameter NH, which specifies the length of the unit hydrograph outflow. However, this is later truncated to the parameter x4 (unless x4 is greater than 20 days (480h), in which case it is truncated to 20 days (480h)). Therefore, in my current opinion, no separate implementation of GR4H is necessary.
The actual evaporation is calculated from the potential evaporation using equation: Calc_Es_V1 based on the filling of the production store. In contrast to the previous evaporation approaches, the net evaporation (i.e. pet - p) is used in the calculation instead of the potential evaporation, for which a suitable submodel interface would first have to be created that also passes on the precipitation or only the net evaporation to the submodel. This should not be too difficult to implement, but requires a little programming effort
gr5j in the air package airGR splits the output of the unit hydrograph, but in some publications gr5j (like gr4j and gr6j) the input to the unit hydrograph is split and two seperate unit hydrographs are applied. Are we sure that we want to stay with the airGR version? Lavenne: airGR:
For equation Calc_SolidFraction_V1 the literature Turcotte2007 is referenced, but no page is given. The document has over 400 pages and no specific page is given so we cannot prove the correctness of this method. For equation Calc_SolidFraction_V1 the threshold of 1500m does not seem to make sense because it leads to a unsteady fuction (see doctest). The difference in the results for 1500m depends strongly on the distribution of Tmin, Tmax and Tmean. It is therefore not possible to find a continuous transition.
Currently all variable names are taken from the airGR package. Should we adjust them. Should we find more descriptive names or leave then as they are.
The equation $$Melt = ((1 - MinMelt) \cdot GRatio + MinMelt) \cdot PotMelt$$ leads to small melt rates for small amounts of snow. In summer the amount of snow is therefore approaching 0, but never reaches 0. Might this be a problem for us and should we fix this?
We assume (similarly to the implementation of hland) that zinputs is not the station height, but always the mean height of the catchment area. Therefore the zinputs parameter is no longer necessary and can be calculated by zlayers and layerarea. We decided to remove the hypsodata option from zlayers and add a pre-processing method to the model to calculate layerarea and zlayers from hypsodata, as layerarea should not be set independently of hypsodata.
I have updated my comments with the results of our discussions yesterday and today.
In the current HydPy-gland version we have a mixture of parameter and series names from:
In the following I want to give an overview which can help us to | HydPy (current) | Fortran | R | documentation | HydPy (proposed) |
---|---|---|---|---|---|
Area | |||||
X1 | Param(1) | x1 | X1 | X1 | |
X2 | Param(2) | x2 | X2 | X2 | |
X3 | Param(3) | x3 | X3 | X3 | |
X4 | Param(4) | x4 | X4 | X4 | |
X5 | Param(5) | x5 | X5 | X5 | |
X6 | Param(6) | x6 | X6 | X6 | |
IMax | Imax | Imax | Imax | IMax | |
P | P1/Precip | Precip | P | P | |
PET | E/PE | PotEvap | E | E | |
En | EN | En | EN | ||
Pn | PN/Pn | Pn | Pn | PN | |
Ps | PS/Ps | Ps | Ps | PS | |
EI | EI | EI | Ei | EI | |
Es | ES | Es | ES | ||
AE | AE | AE | AE | ||
Pr | PR | PR | Pr | PR | |
PrUH1 | PRHU1 | PR9 | |||
PrUH2 | PRHU2 | PR1 | |||
QOutUH2 | StUH2 | Q10 | |||
Perc | PERC/Perc | Perc | Perc | Perc | |
Q9 | StUH1(1)/Q9 | Q9 | Q9 | Q9 | |
Q1 | StUH2(1)/Q1 | Q1 | Q1 | Q1 | |
EXCH/Exch | Exch | F | FPot | ||
FD | AEXCH2/Aech2 | Exch2 | F | FD | |
FR | AEXCH1/Aexch1 | Exch1 | F | FR | |
AEXCH1+AEXCH2/AExch | AExch | FTot | |||
FR2 | EXCH | Exch | F | FR2 | |
Qr | QR | QR | Qr | QR | |
Qr2 | QRExp | QRExp | QrExp | QR2 | |
Qd | QD | QD | Qd | QD | |
Qt | Q/Qsim | Qsim | Q | QH | |
QV | |||||
I | St(3) | Interc | I | I | |
S | St(1)/Prod | Prod | S | S | |
R | St(2)/Rout | Rout | R | R | |
R2 | St(3) | Exp | R2 | R2 |
Feel free to comment and make suggestions
Shouldn't we use for input Sequences like Precipitation the standard HydPy names? I can't find the issue mentioning these names. Is there a standard name for potential evaporation? I can't find the variable mean temperature in the list.
We discussed this in #75. We now support multiple naming conventions. The actual implementation of an InputSequence
subclass (its name) usually follows the model-specific nomenclature. The HydPy standard name is added via the class variable STANDARD_NAME
.
The current list of the standard names is here. We added no "Mean_" prefixes to the names (so far) because reading averages is the standard case while, for example, reading maximums is special.
The potential evapotranspiration standard name is defined here (I overlooked it at first sight. We should better sort all names in strict alphabetical order.)
We still have problems in making our model available in different time steps. Ficchi 2019 (doi:10.1016/j.jhydrol.2019.05.084 ) describes in Table A.1 that the equations for
@BGWKlein implemented prototypes for GR4J and some related models a long time ago, and @sivogel did some polishing a year ago in #112 and recently updated her PR to account for the new submodel concept (regarding evaporation.
It might be a little late, but I open this issue as a notebook to keep track of the final discussions.