Refuse linCmtB(which1 = -3) when linCmt() compartments lag differently - #1272
Merged
Conversation
linCmtB(which1 = -3), the dose-time (moving boundary) sensitivity added in #1235, is the derivative wrt ONE delay applied to every dose feeding the linear system -- valid only when every dosed linCmt() compartment shares the same alag(). A model that lags two linCmt() compartments differently (e.g. alag(depot) and alag(central) driven by distinct parameters) previously built and solved silently to the wrong, single-delay answer. rxode2() now detects this at build time: when the model calls linCmtB() with a (possibly indirect) literal which1 = -3 and its linCmt() compartments carry more than one distinct alag() expression, it errors instead of compiling. The full architectural fix -- per-compartment sensitivity vectors carried through the linCmt solve -- is a much larger change shared with #1236 and is not attempted here. Fixes #1237.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1237.
linCmtB(which1 = -3)(the dose-time / moving-boundary sensitivity added in #1235) is the derivative of alinCmt()model wrt ONE delay applied to every dose feeding the linear system -- valid only when every dosedlinCmt()compartment shares the samealag(). A model that lags twolinCmt()compartments differently (e.g.alag(depot)andalag(central)driven by distinct parameters) previously built and solved silently to the wrong, single-delay answer instead of erroring.The full architectural fix -- per-compartment sensitivity vectors carried through the
linCmtsolve (extendingnumLinSens, thelinCmtFlg/.rxLinCmt()/op->linOffsetplumbing,saveJac/restoreJac) -- is a much larger change explicitly shared with #1236 and is not attempted here. Instead this implements the "meanwhile" guard the issue itself scoped out: refuse to build the offending model instead of silently returning a wrong answer.rxode2()now detects, at build time, when a model callslinCmtB()with a (possibly indirect, via simple constant folding) literalwhich1 = -3while itslinCmt()compartments carry more than one distinctalag()expression, and raises a clear error instead of compiling.alag()assigned conditionally (different text perif/elsebranch) does not by itself trip the guard -- only a mismatch across compartments does.linCmtBheader comment insrc/linCmt.cppto point at the new guard.This underwent an independent review pass (Antigravity/Gemini) that surfaced two real gaps, both fixed and covered by new tests: a
which1value reached indirectly through a constant variable, and a false positive on a single compartment's conditionally-reassignedalag().Test plan
tests/testthat/test-lincmt-dose-time-sens-guard.R(7 tests): errors on distinctalag()across twolinCmt()compartments, allows a sharedalag(), allows distinctalag()s whenwhich1 = -3is never used, allows a single lagged compartment, allows no modeledalag()at all, still errors whenwhich1is reached indirectly through a constant, and allows a single compartment's conditionally-reassignedalag().tests/testthat/test-lincmt-dose-time-sens.R(20 tests, unmodified) still pass.lincmt/event-sensitivities/prior-densityfiltered suite (6730 tests) passes after merging latestmain.