mrc-ide / odin

ᚩ A DSL for describing and solving differential equations in R
https://mrc-ide.github.io/odin
Other
105 stars 13 forks source link

discrete models do not respect `use_dde` argument #246

Open hillalex opened 3 years ago

hillalex commented 3 years ago

To reproduce:

remove.packages("dde")
gen <- odin({
   initial(x) <- 1
   update(x) <- x + 1
})
mod <- gen$new(use_dde=FALSE) # FALSE is the default value anyway, but just to be really clear
mod$run(0:10)

See error:

Error in loadNamespace(name) (odin.R#100): there is no package called ‘dde’

Looks like this code path uses dde unconditionally: https://github.com/mrc-ide/odin/blob/32cabd2e1098fadddc37de0c10d2b81004419790/R/wrapper.R#L175

Note also that dde is only a suggested package, but this makes it a true dependency.

richfitz commented 3 years ago

Yeah, this is not spectacular, but it's been that way forever - most of it reflects the initial nature of discrete time models as bolted on to odin. We'll shortly move to pushing dust as the main way to run them I think.

There's a similar situation with V8 now - if you compile a model to js then you really need V8 in order to work with it. So the current dependency list is the "core" list.

Probably given how simple the usage is here we should probably write a simple iterator for odin models in the meantime

hillalex commented 3 years ago

Just indicating in the docs that dde is a requirement for discrete models would be a quick improvement.

richfitz commented 3 years ago

yeah, that's easy enough to add. Also probably a better error message?

hillalex commented 3 years ago

Yes, I was initially confused and thought I'd done something wrong, until I looked at the source code. A nice odin error message would have reassured me!