Resolve "SD model: add an argument `unit` in CreateInputsModel and store `Qupstream` in m3/time step"

Closes #110 (closed)

changed milestone to %v1.6.11

added ENHANCEMENT label

added 1 commit

• 191d4c70 - feat(SD): Add parameter 'QupstrUnit'

Compare with previous version

added 5 commits

• 191d4c70...a2078fd2 - 4 commits from branch dev
• 67cd10bb - Merge branch 'dev' into...

Compare with previous version

assigned to @olivier.delaigue and unassigned @david.dorchies

marked this merge request as ready

marked this merge request as draft

I have detected an issue on RunModel_Lag which is not taking correctly into account upstream flows. I'm still debugging.

resolved all threads

marked this merge request as ready

added 1 commit

• 6991a504 - docs(man): add missing arguments in RunModel and Calibration_Michel help pages

Compare with previous version

added 1 commit

• 77fa9c32 - refactor: check the QupstrUnit argument using match.arg in CreateInputsModel

Compare with previous version

Is there a problem with the following situation ?

> QupstrUnit <- NULL
> match.arg(arg = QupstrUnit, choices = c("mm", "m3", "m3/s", "l/s", "L/s"))
[1] "mm"

It's maybe exaggerated but we should prevent that a NULL value make a choice be default that it is not known by the user. I suggest to adopt one of these solutions:

• if(is.null(QupstrUnit)) stop("QupstrUnit should be one of \"", paste(choices, collapse = "\", \""), "\"")
• if(is.null(QupstrUnit)) warning("'QupstrUnit' has been redefined to \"mm\"")

Using tolower(QupstrUnit) fixed it and it allows to be flexible in the respect of the case of characters.

> match.arg(arg = tolower(NULL), choices = c("mm", "m3", "m3/s", "l/s"))
Error in match.arg(arg = tolower(NULL), choices = c("mm", "m3", "m3/s",  : 
  'arg' doit être un de “mm”, “m3”, “m3/s”, “l/s”
> match.arg(arg = NULL, choices = c("mm", "m3", "m3/s", "l/s"))
[1] "mm"

I wasn't shocked by the previous behavior, because that's how the match.arg() function usually works.

And it is a normal behavior in R to return the default value if the NULL value is used.

added 1 commit

• 7cbab175 - refactor(test): manage with case sensitivity of the QupstrUnit argument in...

Compare with previous version

added 1 commit

• 5008f056 - test: fix the use of quotes to check the QupstrUnit argument of CreateInputsModel

Compare with previous version

added 1 commit

• 5859baed - style(test): use a lower case to write the exponant values i order to homogenize the code

Compare with previous version

added 1 commit

• 9a98522c - refactor: manage with case sensitivity of the QupstrUnit argument in CreateInputsModel

Compare with previous version

mentioned in issue #110 (closed)

marked this merge request as draft

added 1 commit

• 5d52933b - fix: Qupstream conversion error with several upstream flows

Compare with previous version

marked this merge request as ready

mentioned in commit db128c87

merged