airGR issueshttps://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues2020-07-22T09:51:25+02:00https://gitlab.irstea.fr/HYCAR-Hydro/airgr/-/issues/60Migrate TransfoParam functions and CreateCalibOptions to S3 methods2020-07-22T09:51:25+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 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)`.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/59Add comparative tests with master branch on examples2020-07-15T10:51:27+02:00Dorchies DavidAdd comparative tests with master branch on examplesTODO:
* add a script in order to install airGR package from cran, browse manual files and for each run the example and record all the variables created by each example. This record will be a file saved in artefact for use in next CI jobs. This script should be notices in `.Rbuildignore`.
* Modify `.gitlab-ci.yml` this script should be run before build and test of the package.
* Add test that run the examples and compare the variables with the one coming from the example executed from the CRAN version package.TODO:
* add a script in order to install airGR package from cran, browse manual files and for each run the example and record all the variables created by each example. This record will be a file saved in artefact for use in next CI jobs. This script should be notices in `.Rbuildignore`.
* Modify `.gitlab-ci.yml` this script should be run before build and test of the package.
* Add test that run the examples and compare the variables with the one coming from the example executed from the CRAN version package.v2.0Dorchies DavidDorchies David