airGR issueshttps://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues2023-10-26T11:23:40+02:00https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/176CreateIniStates: Huge computation time spent on GetFeatModel2023-10-26T11:23:40+02:00Dorchies DavidCreateIniStates: Huge computation time spent on GetFeatModelThe Rstudio profiler on airGRiwrm calibration with ungauged nodes returns this result:
![image](/uploads/2a1ef7ba9e8378e269332851c470569c/image.png)
We see that on 3.46 sec of calculation, 1.62 sec are spent in `CreateIniStates` and es...The Rstudio profiler on airGRiwrm calibration with ungauged nodes returns this result:
![image](/uploads/2a1ef7ba9e8378e269332851c470569c/image.png)
We see that on 3.46 sec of calculation, 1.62 sec are spent in `CreateIniStates` and especially in the call to `.GetFeatModel` which take a lot of time to run `system.file` and `read.table`.
One first quick solution would be to add in `Calibration`, a copy of model features from `RunOptions` to `InputsModel` in order to provide them to `CreateIniStates` through `InputsModel`.
If we think of more structured solution, model features could be attached from the beginning to the `InputsModel` object and propagated to all subsequent objects used in simulation/calibration without the need to call again the expensive `.GetFeatModel` function. This point of view change substantially the call to airGR functions because we would need to precise `IsSD` and `IsHyst` parameters in the `CreateInputsModel` call. This also would simplify the calls to other `Create...` functions which would not need these parameters anymore.
@guillaume.thirel, @olivier.delaigue, if you agree with this point of view, this proposed solution needs further investigation in order to avoid breaking changes and take into account any legacy and new call to airGR functions. Maybe in a separated ticket issue ?v1.8https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/169Fix GR pathology2023-10-26T11:24:06+02:00Thirel GuillaumeFix GR pathologyIssue initially reported to Vazken and Charles by Wouter Knoben on the Excel GR4J in 2017, also reported recently by @laurent.strohmenger on GR5J recently.
Note: this issue concerns airGR but is not specific to the package, it comes fr...Issue initially reported to Vazken and Charles by Wouter Knoben on the Excel GR4J in 2017, also reported recently by @laurent.strohmenger on GR5J recently.
Note: this issue concerns airGR but is not specific to the package, it comes from the GR implementation.
Under some conditions (i.e. some parameters sets, some climatic conditions), the lower part of the GR models, namely the routing store and the exchange component, enter in a mode where it cannot escape anymore. Due to a positive X2 and a positive exchange larger than the routing store outflow, the routing store comes to an equilibrium and a non null discharge is produced, even though no more rainfall is injected.
This is very annoying, as the simulation quality becomes very poor. This issue may not be found under calibration, because a low objective function would lead to avoid these parameter sets, but some parameter sets can look ok under calibration and then give erroneous simulations, under a different climate for instance (e.g. high rainfall).
Included is the email exchange with Wouter Knoben colleagues.
[GR4J_pathology_Knoben.docx](/uploads/715ff50a3c0232a3125178db848fdc15/GR4J_pathology_Knoben.docx)
Attached also an R code reproducing the bug proposed by @laurent.strohmenger and augmented by the translation in R of the GR5J routing part producing the bug. It shows that X2 does not need to be larger than X3, oppositely to what was discussed in 2017.
[GR5J_bug.R](/uploads/4fc1d93321a41a052546d69d9b435427/GR5J_bug.R)
There is no solution to date, but this must be cured.v2.0https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/166Update GR5H interception grid-screening parameter quantiles2022-09-13T14:58:36+02:00Delaigue OlivierUpdate GR5H interception grid-screening parameter quantilesAccording to the tests made by @cyril.thebault on the ~579 catchments, it seems to be needed to update the GR5H interception parameter quantiles used during the grid-screening calibration step.
Current values:
```r
GR5Hinterception = ...According to the tests made by @cyril.thebault on the ~579 catchments, it seems to be needed to update the GR5H interception parameter quantiles used during the grid-screening calibration step.
Current values:
```r
GR5Hinterception = matrix(c(+3.46, -1.25, +4.04, -9.53, -9.34,
+3.74, -0.41, +4.78, -8.94, -3.33,
+4.29, +0.16, +5.39, -7.39, +3.33), ncol = 5, byrow = TRUE)
```
New values (found after 4 convergence loops):
```r
GR5Hinterception = matrix(c(+4.92, -0.17, +4.27, -9.81, -9.29,
+5.42, -0.07, +4.94, -9.66, -7.36,
+6.05, -0.01, +5.63, -9.26, -5.55), ncol = 5, byrow = TRUE)
```v1.8https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/161RunModel_Lag: proposal for a better transformation and screening of the param...2023-02-02T14:04:02+01:00Dorchies DavidRunModel_Lag: proposal for a better transformation and screening of the parametersThe work of Enola Henrotin highlights some issues with the calibration of the lag model. These issues are essentially due to an inadequate transformation of the celerity parameter.
The document below summarizes these issues and proposes...The work of Enola Henrotin highlights some issues with the calibration of the lag model. These issues are essentially due to an inadequate transformation of the celerity parameter.
The document below summarizes these issues and proposes a new transformation and a set of screening values from experimental measurements of flow velocities in a various rivers:
[Sensibility-of-celerity-parameter-on-Lag-model.html](/uploads/ea137265ad43a30113926721c4497ca1/Sensibility-of-celerity-parameter-on-Lag-model.html)
The proposed formulas are (respectively for the transformed and the real parameter):
```math
C_{T} = \dfrac{120}{299C_{0}} - \dfrac{3010}{299} \\
C_0 = \dfrac{120}{299 (C_T + 3010 / 299)}
```
The proposed screening values of transformed parameter are: -9.7; -8.7; -2.0
Sources of the document:
- [Sensibility_of_celerity_parameter_on_Lag_model.Rmd](/uploads/504b8cafa8763657fa1ee9036dbd9349/Sensibility_of_celerity_parameter_on_Lag_model.Rmd)
- [celerity_sensibility.bib](/uploads/484d9de749caec8781c72fb71ee774a1/celerity_sensibility.bib)v1.8https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/155Add the names of the parameter in Calibration2022-07-18T17:39:08+02:00Dorchies DavidAdd the names of the parameter in CalibrationAdd a names to the final parameters with the complete name of the parametersAdd a names to the final parameters with the complete name of the parametersv1.8https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/153SD: Add Linear Lag and Route propagation model2023-10-26T11:24:53+02:00Dorchies DavidSD: Add Linear Lag and Route propagation modelProcessed by Enola Henrotin.
See the forked project on https://gitlab.irstea.fr/david.dorchies/airgrProcessed by Enola Henrotin.
See the forked project on https://gitlab.irstea.fr/david.dorchies/airgrv2.0Dorchies DavidDorchies Davidhttps://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/152SD: Add capability to integrate other propagation models2023-10-26T11:24:58+02:00Dorchies DavidSD: Add capability to integrate other propagation modelsAs for `FUN_MOD` for choosing the GR model, add a `FUN_SD` parameter to allow the user to choose the propagation model in SD models.
This work is processed by Enola Henrotin on the fork https://gitlab.irstea.fr/david.dorchies/airgrAs for `FUN_MOD` for choosing the GR model, add a `FUN_SD` parameter to allow the user to choose the propagation model in SD models.
This work is processed by Enola Henrotin on the fork https://gitlab.irstea.fr/david.dorchies/airgrv2.0Dorchies DavidDorchies Davidhttps://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/150Implemente Daniela Peredo's work2022-07-19T08:50:53+02:00Thirel GuillaumeImplemente Daniela Peredo's workThat could be useful to add Daniela's work in airGR (see https://www.tandfonline.com/doi/full/10.1080/02626667.2022.2030864).
Maybe @paul.astagneau you have already implented it by the way?That could be useful to add Daniela's work in airGR (see https://www.tandfonline.com/doi/full/10.1080/02626667.2022.2030864).
Maybe @paul.astagneau you have already implented it by the way?https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/143SeriesAggreg: allow to define a unique aggregation function for multiple columns2021-12-16T08:22:17+01:00Dorchies DavidSeriesAggreg: allow to define a unique aggregation function for multiple columns`SeriesAggreg` is very useful for calculating annual or monthly indicators.
For example, I use it to calculate annual hydrologic indicator on data.frame containing the simulated flow for plenty of gauging stations.
For doing that, I ha...`SeriesAggreg` is very useful for calculating annual or monthly indicators.
For example, I use it to calculate annual hydrologic indicator on data.frame containing the simulated flow for plenty of gauging stations.
For doing that, I have to write:
```r
SeriesAggreg(dfQ, Format = "%Y", ConvertFun = rep("calcMyIndicator", ncol(dfQ) - 1))
```
Could we assume that if a unique function is provided by the user for `ConvertFun` therefore it is implicitly applied for all columns?
This syntax would be largely more convenient:
```r
SeriesAggreg(dfQ, Format = "%Y", ConvertFun = "calcMyIndicator")
```https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/140Adjust TransfoParam_GR5H (X2)2023-10-26T11:26:08+02:00Astagneau PaulAdjust TransfoParam_GR5H (X2)There might be an issue with the transformation of X2 in GR5H.
The transformation of X2 (from T to R) in `TransfoParam_GR5J` and `TransfoParam_GR5H` is:
`ParamOut[, 2] <- sinh(ParamIn[, 2])`
For both models, the calculation of potentia...There might be an issue with the transformation of X2 in GR5H.
The transformation of X2 (from T to R) in `TransfoParam_GR5J` and `TransfoParam_GR5H` is:
`ParamOut[, 2] <- sinh(ParamIn[, 2])`
For both models, the calculation of potential intercatchment semi-exchange is:
`EXCH=Param(2)*(St(2)/Param(3)-Param(5))`
X2 is in mm/timestep, which means that X2 is timestep dependant.
The calculation of hourly potential intercatchment semi-exchange is calculated differently by [Le Moine, 2008, p. 173](https://webgr.inrae.fr/wp-content/uploads/2012/07/2008-LE_MOINE-THESE.pdf).
The distribution of X2 over 229 catchments changes when dividing X2 by 24 in the fortran code. ![X2change_distrib_2021-12-07](/uploads/207f5b4cef5917388fa6a41c28371b40/X2change_distrib_2021-12-07.png)
Most X2 values were equal to -0.04 mm/h, which is one of the prefiltering values.v1.8https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/135Regularisation: handle composite criterion2021-11-03T14:22:18+01:00Dorchies DavidRegularisation: handle composite criterionTest and implement the possibility to use composite criteria (e.g.: KGE(sqrt(Q)) mixed with KGE(1/Q)) with Lavenne regularisation.Test and implement the possibility to use composite criteria (e.g.: KGE(sqrt(Q)) mixed with KGE(1/Q)) with Lavenne regularisation.v1.8Dorchies DavidDorchies Davidhttps://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/118plot.OutputsModel: remove case sensitivity to the `which` argument2021-04-26T14:35:21+02:00Dorchies Davidplot.OutputsModel: remove case sensitivity to the `which` argumentIs it possible to have the `which` argument case insensitive?:
```r
> plot(OMinf2nat$CHALO_21, Qinf[I_Run, "CHALO_21"], which = "regime")
Error in plot.OutputsModel(OMinf2nat$CHALO_21, Qinf[I_Run, "CHALO_21"], :
incorrect element fo...Is it possible to have the `which` argument case insensitive?:
```r
> plot(OMinf2nat$CHALO_21, Qinf[I_Run, "CHALO_21"], which = "regime")
Error in plot.OutputsModel(OMinf2nat$CHALO_21, Qinf[I_Run, "CHALO_21"], :
incorrect element found in argument 'which': "regime"
it can only contain "all", "synth", "ts", "perf", "Precip", "PotEvap", "ActuEvap", "Temp", "SnowPack", "Flows", "Error", "Regime", "CumFreq", "CorQQ"
> plot(OMinf2nat$CHALO_21, Qinf[I_Run, "CHALO_21"], which = "Regime")
```https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/117Check the compatibility between the RunOptions and the InputsModel obects2021-11-03T14:24:36+01:00Dorchies DavidCheck the compatibility between the RunOptions and the InputsModel obectsHere the necessary data: [GR4J_crash.RData](/uploads/8f44ff6a10af128a396c78110b9a7e6e/GR4J_crash.RData)
And the code tested on the last `dev` version:
```r
> library(airGR)
> load("GR4J_crash.RData")
> ls()
[1] "InputsModel" "Param" ...Here the necessary data: [GR4J_crash.RData](/uploads/8f44ff6a10af128a396c78110b9a7e6e/GR4J_crash.RData)
And the code tested on the last `dev` version:
```r
> library(airGR)
> load("GR4J_crash.RData")
> ls()
[1] "InputsModel" "Param" "RunOptions"
> Param
[1] 169.017118 -2.375568 20.697233 1.417417
> RunModel_GR4J(InputsModel, RunOptions, Param)
Error in RunModel_GR4J(InputsModel, RunOptions, Param) :
NA/NaN/Inf in foreign function call (arg 2)
```
The error occurs in the Fortran call. I don't know how to debug that...v1.8Delaigue OlivierDelaigue Olivierhttps://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/97Parallelize CemaNeige2021-03-02T09:59:34+01:00Thirel GuillaumeParallelize CemaNeigeSimilarly to https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/96, the computation done on several altitude bands of CemaNeige could be parallelized. This remark is potentially valid both for `RunModel_CemaNeige*` calculation but also ...Similarly to https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/96, the computation done on several altitude bands of CemaNeige could be parallelized. This remark is potentially valid both for `RunModel_CemaNeige*` calculation but also for `DataAltiExtrapolation_Valery`.https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/96Parallelize the grid-screening step during calibration2021-11-03T14:13:16+01:00Delaigue OlivierParallelize the grid-screening step during calibrationIt is possible to parallelize the grid-screening step during calibration, this would speed up the calibration when the model used has many parameters, especially for some 'airGRplus' models.
We can use the 'parallel' package in order no...It is possible to parallelize the grid-screening step during calibration, this would speed up the calibration when the model used has many parameters, especially for some 'airGRplus' models.
We can use the 'parallel' package in order not to depend to an external package, because this one is automatically installed with R.v1.8https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/84Add the use of '['.OutputsModel in plot.OutputsModel2021-07-14T10:14:45+02:00Delaigue OlivierAdd the use of '['.OutputsModel in plot.OutputsModelIt could be nice to simplify the code of the `plot.OutputsModel()` function by the use of '['.OutputsModel in order to manage the `IndPeriod_Plot` argument.It could be nice to simplify the code of the `plot.OutputsModel()` function by the use of '['.OutputsModel in order to manage the `IndPeriod_Plot` argument.v2.0https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/60Migrate TransfoParam functions and CreateCalibOptions to S3 methods2021-06-03T11:50:21+02:00Dorchies DavidMigrate TransfoParam functions and CreateCalibOptions to S3 methodsThe main idea is to facilitate the model chaining (e.g. CemaNeige + GR4J + LAG) in order to reduce the current code complexity and facilitate extension to new models in the future. S3 class concepts are a good candidate to tackle this i...The main idea is to facilitate the model chaining (e.g. CemaNeige + GR4J + LAG) in order to reduce the current code complexity and facilitate extension to new models in the future. S3 class concepts are a good candidate to tackle this issue. A Proof of Concept written in Rmarkdown showing how model chaining can be operated is available here :
[PoC_airGR_S3_classes.Rmd](/uploads/4492a10c93c3c7ba4547c76bf6f19896/PoC_airGR_S3_classes.Rmd)
## Starting point
Currently in `master` and `dev` branches, depending on the `FUN_MOD` provided to `CreateCalibOptions` a serie of conditions assign a `FUN1` variable corresponding to a `TransfoParam_GR*` function and a `FUN2` variable corresponding to either `TransfoParam_CemaNeige` or `TransfoParam_CemaNeigeHyst` depending on `isHyst` parameter. If several models are involved (e.g. CemaNeige + GR4J), then a "meta" FUN_TRANSFO function is written, binding columns of the output matrix with outputs of each model. In the `SD` branch the complexity of the process is again complicated by adding parameters of the LAG model.
## Proposition
- Modify CreateCalibOption in order to assign classes corresponding to the models involved in the model chain to the and call a generic function `TransfoParam`
- Change the name of `transfoParam_*` function to `TransfoParam.*` methods
- Rewrite `TransfoParam` generic function in order to automatically bind columns of the matrix `ParamT`
## Pitfalls
If we assume a generic form of the model chaining and put in correspondence the order of the model with the order of the parameters, the current order in `SD` implementation is not compliant: the current order is : `param = c(GRparam, CemaneigeParam, LAGparam)`. If we considere `Cemaneige` as a pretreatment of the rain, before running a `GR` model and `LAG` a routing model applied after GR model, this order of parameter is not logical.
To solve this issue, as the order of parameter between `GR` and `Cemaneige` is already fixed on older versions and SD model is not public yet, I propose to adopt this order of parameters: `param = c(LAGparam, GRparam, CemaneigeParam)`.v2.0Dorchies DavidDorchies Davidhttps://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/54Add a dam module with the lumped GR models2021-04-27T08:11:42+02:00Thirel GuillaumeAdd a dam module with the lumped GR modelsFollowing the PhD thesis of Jean-Luc Payan and the work of Morgane Terrier, we have pieces of `airGR `codes that allow to take into account the impact of dams in the GR4J and GR5J models. Knowing a time series of delta V, i.e. the variat...Following the PhD thesis of Jean-Luc Payan and the work of Morgane Terrier, we have pieces of `airGR `codes that allow to take into account the impact of dams in the GR4J and GR5J models. Knowing a time series of delta V, i.e. the variation of the volume of water in a dam, it is possible to subtract water from the production store and to release in the the routing store.
Not sure about GR6J, Morgane told me it caused issues with the exponential store, to be checked with @charles.perrin .
Eventual inclusion of this module in `airGR `should be thought of considering parallel works on semi-distribution.
Also important will be how to deal with this additional input data (observed delta V time series).https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/48Improve computation time of the Imax function2021-04-19T08:20:53+02:00Delaigue OlivierImprove computation time of the Imax functionComputation time of the `Imax()` function needs to be improved.The use of the double loop slows down the code strongly.Computation time of the `Imax()` function needs to be improved.The use of the double loop slows down the code strongly.https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/42Add new function to manage time steps2021-04-19T09:09:56+02:00Delaigue OlivierAdd new function to manage time stepsIt is maybe a good idea to create an internal function to manage the time steps in order to avoid command lines in many functions:
```
if ("daily" %in% class(XXXXXXX)) {
TimeStep <- 60 * 60 * 24
}
if ("hourly" %in% class(XXXXXXX)) {
...It is maybe a good idea to create an internal function to manage the time steps in order to avoid command lines in many functions:
```
if ("daily" %in% class(XXXXXXX)) {
TimeStep <- 60 * 60 * 24
}
if ("hourly" %in% class(XXXXXXX)) {
TimeStep <- 60 * 60
}
```