search

nlmixr2 / rxode2

rxode2
https://nlmixr2.github.io/rxode2/
GNU General Public License v3.0
28 stars 8 forks source link

Add model name again #93

Closed mattfidler closed 2 years ago

mattfidler commented 2 years ago

https://github.com/nlmixr2/nlmixr2targets/issues/4

billdenney commented 2 years ago

Is it needed/used? It seems simpler to leave it out. I was asking in nlmixr2targets to understand if I should keep or omit the test.

mattfidler commented 2 years ago

It is used for determining a "sane" name for monolix runs.

It is fine to not include i in the targets test because the name should not affect the results. Simply hash the "$fun" to see if the model changed (though it includes an environment so it may not hash as you expect).

billdenney commented 2 years ago

Ah, okay. In targets, I need to make sure that the digest doesn't change without an important change to the model. (And similarly, that the output results wouldn't change due to a change in the model that doesn't.) The reason that I test for it is because I set it as one of the things that I found changing in nlmixr. If there is a minimal set of things to test for (and generally to make a digest method for), I think that is the goal.

mattfidler commented 2 years ago

Since this a moving target probably a md5 method would be wise. If you open an issue in rxode2 I will look at a minimal set of elements for a hash here and add the $md5 method

mattfidler commented 2 years ago

You would have to hash the control and table options. I was also thinking of a rxControl() option to make changing ode solving options easier.

billdenney commented 2 years ago

Does $md5 have independent value in the object? I ask because I think (and am confirming: ropensci/targets#787) that a digest() is the way to go. If nlmixr2targets is the only reason to add it, then can we switch to having digest() methods?

billdenney commented 2 years ago

Will Landau confirmed that digest() is used for hashes in targets. Can we add a digest() method for nlmxir()-created objects and with that, if the only purpose of the $md5 value is for a digest(), should we remove it?