Repository navigation
fix: keep derived values real-valued in f64 buffers - #372
Merged
Merged
Conversation
Derived and output symbols took their type from the expression assigned to
them, so a whole-number initialiser narrowed them to Int. Their storage is the
f64 derived buffer, so the value was written as a double and read back as an
integer: `z1 = 1.0` then `out(cp) = z1` returned the bit pattern of 1.0
(4.607e18) instead of 1.0.
A conditional with whole-number branches hit the same path:
z1 = if (p1 > 0.0) 1.0 else 2.0
out(cp) = z1
Derived and output symbols are now registered as real-valued, which matches the
buffer they live in. Whole-number constants and parameter-derived values are
unaffected.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #372 +/- ##
==========================================
- Coverage 84.75% 84.75% -0.01%
==========================================
Files 78 78
Lines 35818 35818
==========================================
- Hits 30357 30356 -1
- Misses 5461 5462 +1
🚀 New features to boost your workflow:
|
mhovd
approved these changes
Sep 15, 2026
Merged
Contributor
|
Bencher Report| Project | pharmsol |
| Branch | fix/constant-derived-slots |
| Testbed | mhovd-pgx |
🐰 View full continuous benchmarking report in Bencher
⚠️ WARNING: Truncated view!The full continuous benchmarking report exceeds the maximum length allowed on this platform.
Siel
added a commit
to LAPKB/Pmetrics
that referenced
this pull request
Sep 15, 2026
Picks up the derived-value type fix (LAPKB/pharmsol#372): a derived value initialised with a whole number was typed Int while stored in the f64 derived buffer, so reading it returned the bit pattern. The hoisted helpers this branch introduces rely on that fix. pharmsol 0.29.3 pins its sibling crates exactly, so pharmsol-dsl and pharmsol-macros move with it. pmcore 0.29.0 requires `pharmsol = "0.29.2"` (a caret), so no pmcore change is needed and pmcore stays at 0.29.0.
mhovd
pushed a commit
to LAPKB/Pmetrics
that referenced
this pull request
Sep 16, 2026
* feat: Upgrade to PMcore 0.29 Pin pmcore =0.29.0, which brings pharmsol 0.29.2. ODE dose inputs and their scales now stay on the derivative RHS instead of being declared as separate routes; analytical models keep explicit routes, lag and fa. - `R[j]`/`rateiv[j]` and `B[j]`/`bolus[j]` may be a standalone additive term or one exact product with a scale, in either operand order. - A bolus scale may not depend on state or dose inputs. - A PM_model rebuilt from another PM_model renders its DSL from `arg_list` rather than inheriting a previously rendered string, which could be stale. - Regenerate data/*.rda, which the data-currency test requires after any change to the R6 method bodies. * feat: read like R, and explain what cannot be translated Model equations now behave like R wherever the DSL can follow, and every rejection names the call and the fix rather than generated DSL text. Expressions - Conditionals work in any position. The DSL accepts one only as a whole right-hand side, so nested conditionals are hoisted into derived helpers. - pmin/pmax, variadic max/min, ifelse, log(x, base), trunc and sign are translated instead of rejected. - round(x, digits) follows R's half-to-even rule, not the DSL intrinsic, which rounds halves away from zero. - estimate_effect_2/estimate_effect_3 are available with a checked signature. The retired six-argument get_e2 spelling gets a migration message that states the internal w = alpha * u * v convention. - Unsupported calls quote the R expression. Every unknown function previously crashed with `subscript out of bounds`, so the message written for it was never reached. - Missing and non-finite values are rejected with a fix. - A dose input inside a conditional is refused with the supported alternative. Diagnostics - compile() validates the rendered source, so an invalid model fails where it is defined instead of at the start of a fit. - Every emitted line records the R statement behind it, so backend diagnostics name that statement. - The fit-time note about generated line numbers is removed: it is no longer true. Requires pharmsol with the derived-value type fix (LAPKB/pharmsol#372): a derived value initialised with a whole number was typed Int while stored in an f64 buffer, so hoisted helpers returned the bit pattern instead of the value. data/*.rda is regenerated, which the data-currency test requires after a change to the R6 method bodies. * chore: build against pharmsol 0.29.3 Picks up the derived-value type fix (LAPKB/pharmsol#372): a derived value initialised with a whole number was typed Int while stored in the f64 derived buffer, so reading it returned the bit pattern. The hoisted helpers this branch introduces rely on that fix. pharmsol 0.29.3 pins its sibling crates exactly, so pharmsol-dsl and pharmsol-macros move with it. pmcore 0.29.0 requires `pharmsol = "0.29.2"` (a caret), so no pmcore change is needed and pmcore stays at 0.29.0.
Merged
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
3 participants
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.
A derived value initialised with a whole number was typed
Intfrom its assignment, while derived storage is the f64 buffer. The value was therefore written as a double and read back as an integer, returning the f64 bit pattern instead of the value:Derived and output symbols are now registered as real-valued, matching the buffer they live in. Parameter-derived and fractional-constant values were already typed
Realand are unaffected.Found while adding conditional-desugaring support to Pmetrics, where a hoisted helper such as
_pmt1 = if (c) 1.0 else 2.0produced garbage.tests/dsl_constant_derived.rscovers whole-number, fractional and conditional forms. Full workspace suite: 938 passed, 0 failed.