Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upi161: Allow use of deriv in delay models #162
Conversation
This turns out to segfault which is unpleasant
|
Looks like Appveyor is unhappy for some reason
Workaround is apparently to close and reopen ¯\_(ツ)_/¯ |
Codecov Report
@@ Coverage Diff @@
## master #162 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 38 38
Lines 4474 4500 +26
=====================================
+ Hits 4474 4500 +26
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #162 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 38 38
Lines 4474 4501 +27
=====================================
+ Hits 4474 4501 +27
Continue to review full report at Codecov.
|
|
Sorry this has taken a while, this is first time I've read through any of this code. I feel I still don't understand all of these changes but the bits I do look good. One small question about the testing but I expect that is just something |
| "Can't call deriv() on delay models", fixed = TRUE) | ||
| deriv_c <- mod_c$deriv(t0, mod_c$initial(t0)) | ||
| deriv_r <- mod_r$derivs(t0, mod_c$initial(t0)) | ||
| expect_equal(deriv_c, deriv_r[[1L]], check.attributes = FALSE) |
r-ash
Apr 10, 2019
Contributor
Is there any reason you choose not to use expect_equivalent here?
Is there any reason you choose not to use expect_equivalent here?
richfitz
Apr 10, 2019
Author
Member
Mostly I forget that it exists - have you been using it a lot? I don't see it in dettl
Mostly I forget that it exists - have you been using it a lot? I don't see it in dettl
r-ash
Apr 10, 2019
Contributor
I think more in specio stuff particularly where I'm comparing output from Jeff's code and mine
I think more in specio stuff particularly where I'm comparing output from Jeff's code and mine
richfitz
Apr 10, 2019
Author
Member
cool, that's good to know - I've updated the usages here, which adds a little noise to the diff but it's best if we're using the same things for the same purpose
cool, that's good to know - I've updated the usages here, which adds a little noise to the diff but it's best if we're using the same things for the same purpose
This PR allows the
deriv()method to be used with models that include delays.Previously this was disabled because we need to be careful about treatment of the history. However, it's not too hard to do something along the lines of the Right Thing:
deriv()should treat this as if it's still burning the delays in (i.e., use the initial conditions)deriv()should not affect the history (especially once model resuming is implemented - #141) so we should just use the history stored in the modelIn order to support this:
createphase initialises theinitial_tmember toNA(previously was default-initialised to zero which is not ideal)initial_telement to the same time as current so that we always compute the delay-free derivatives - on exit we reset theinitial_tto zero.Most of the PR is a contrived test case that checks this is all reasonable.