Calvagone / campsis

A generic PK/PD simulation platform based on rxode2 and mrgsolve engines.
https://calvagone.github.io/
GNU General Public License v3.0
8 stars 3 forks source link

Covariate name should be trimmed #137

Closed luyckxn closed 8 months ago

luyckxn commented 8 months ago

If a covariate name has a trailing space, it may not be detected by mrgsolve as an error (see test in this ticket), while rxode2 will raise a clear error. In that case, with mrgsolve, the covariate in the model (without trailing space) is not initialised correctly... Very frustrating for the user.

Suggestion: remove leading and trailing spaces automatically in campsis to avoid such issues.

kylebaron commented 8 months ago

@luyckxn Please let me know if you think mrgsolve isn't handling things properly here. We don't trim any covariate names but I'm not sure where this might be an issue (haven't ran into problems with this before).

luyckxn commented 8 months ago

@kylebaron: this is a very minor issue. If the dataset you give to mrgsolve has a covariate column, written with a trailing space, mrgsolve does not throw an error (while usually if a covariate is missing, you have a clear error message)

Here is an example with Campsis code:

library(campsis) # Campsis before this fix

model <- model_suite$pk$'1cpt_fo' %>%
  replace(Equation("CL", "TVCL * exp(ETA_CL) *pow(BW/70, 0.75)"))

# 1) BW in dataset correctly written
dataset <- Dataset(1) %>%
  add(Bolus(time=0, amount=1000)) %>%
  add(Observations(0:24)) %>%
  add(Covariate("BW", 70))

# Same results
results <- simulate(model=model %>% disable("IIV"), dataset=dataset, dest="mrgsolve")
spaghettiPlot(results, "CONC")

results <- simulate(model=model %>% disable("IIV"), dataset=dataset, dest="rxode2")
spaghettiPlot(results, "CONC")

# 2) BW in dataset with trailing space
dataset <- Dataset(1) %>%
  add(Bolus(time=0, amount=1000)) %>%
  add(Observations(0:24)) %>%
  add(Covariate("BW ", 70))

# No error, wrong results (CL=0)
results <- simulate(model=model %>% disable("IIV"), dataset=dataset, dest="mrgsolve")
spaghettiPlot(results, "CONC")

# Clear error message: The following parameter(s) are required for solving: BW
results <- simulate(model=model %>% disable("IIV"), dataset=dataset, dest="rxode2")
spaghettiPlot(results, "CONC")
kylebaron commented 8 months ago

Thanks, @luyckxn. It seems reasonable for Covariate() to trim the names as part of that interface. mrgsolve will take the names as provided by the user. mrgsolve wouldn't trim the names because it would be unusual to have this sort of input created with plain R or the functionality exported by mrgsolve.

There are a couple of mechanisms to either get mrgsolve to error (use the $INPUT block with check_data_names()) or to manually check that the name passed to Covariate() is indeed a covariate (just check that "BW" is in names(param(mod)) would be the most simple approach.

Best regards, Kyle