Feat/data mapping routes#254
Open
Siel wants to merge 11 commits intofeat/add-missing-macrosfrom
Open
Conversation
…rings instead of Integer. This gives flexibility to modelers to define routes in terms of indices or named elements. All three surfaces for Model creation are also updated (macros, SDL, and ::new() constructors)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the DSL + Rust macro modeling surfaces to support richer route/output metadata (including route kind + shared-channel semantics), introduces string-based channel identifiers for events, and aligns authoring syntax changes (name = ..., structure = ...) across tests, examples, and runtime backends.
Changes:
- Add/propagate validated model metadata (parameters/covariates/states/routes/outputs; route kind + channel sharing) through ODE/Analytical/SDE equations and runtime backends.
- Switch event channel identifiers (inputs/outputs) from
usizeto aChannelIdwrapper, enabling both named and numeric labels. - Update DSL authoring syntax (
model→name,kernel→structure) and extend parsing/lowering to preserve route kind and allow numeric output labels.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/support/runtime_corpus.rs | Updates DSL fixtures to name/structure and adds full-feature ODE/Analytical runtime corpus cases. |
| tests/support/bimodal_ke.rs | Updates authoring DSL fixture to name = .... |
| tests/runtime_backend_matrix.rs | Adds backend-matrix tests for new full-feature ODE/Analytical corpus cases. |
| tests/full_feature_macro_parity.rs | Adds parity tests comparing full-feature macro models vs handwritten models (metadata + predictions). |
| tests/browser-e2e/site/app.mjs | Updates embedded DSL sources to name = .... |
| tests/analytical_macro_lowering.rs | Adds tests validating analytical! macro lowering vs handwritten behavior across features. |
| src/test_fixtures.rs | Updates fixture syntax to structure = .... |
| src/simulator/equation/sde/mod.rs | Adds optional validated metadata + route mapping behavior to SDE and refactors event handling for ChannelId. |
| src/simulator/equation/ode/mod.rs | Adds optional validated metadata to ODE and resolves inputs/outputs via ChannelId-aware event processing. |
| src/simulator/equation/ode/closure.rs | Updates infusion schedule handling to resolve numeric input indices from ChannelId. |
| src/simulator/equation/mod.rs | Renames meta→metadata, introduces metadata-aware label resolution and occasion event resolution. |
| src/simulator/equation/meta.rs | Removes the old Meta container in favor of the new metadata system. |
| src/simulator/equation/analytical/mod.rs | Adds optional validated metadata to Analytical and updates infusion/output handling for ChannelId. |
| src/lib.rs | Re-exports new metadata APIs and macros (analytical, ode, sde) via crate root + prelude. |
| src/error/mod.rs | Adds explicit UnknownInputLabel / UnknownOutputLabel errors for label resolution failures. |
| src/dsl/wasm_direct_emitter.rs | Updates route input load shape to new ExecutionLoad::RouteInput { .. } form. |
| src/dsl/wasm_compile.rs | Updates embedded DSL sources to name = ... in wasm compilation tests. |
| src/dsl/wasm.rs | Extends native route info with declaration_index and optional route kind. |
| src/dsl/rust_backend.rs | Updates route input load shape to new ExecutionLoad::RouteInput { .. } form. |
| src/dsl/native.rs | Adds route kind semantics + label resolution for native runtime models; improves event resolution and derived refresh. |
| src/dsl/model_info.rs | Tracks route declaration order/indices + kind; updates injection detection for shared channels and adds tests. |
| src/dsl/jit.rs | Updates route input load lowering to new ExecutionLoad::RouteInput { .. } form and adds shared-channel test. |
| src/dsl/compiled_backend_abi.rs | Updates ABI tests for extended route info fields. |
| src/data/structs.rs | Changes output equation collection to ChannelId; updates expansion logic and lag/fa application guards for numeric parsing. |
| src/data/row.rs | Switches DataRow input/outeq to ChannelId and updates builder APIs to accept string-like labels. |
| src/data/parser/pmetrics.rs | Parses INPUT/OUTEQ as strings into ChannelId (preserving named labels) and adds regression tests. |
| src/data/event.rs | Introduces ChannelId and migrates bolus/infusion/observation labels from usize to ChannelId. |
| src/data/builder.rs | Updates SubjectBuilder APIs to accept string-like labels for input/output channels. |
| pharmsol-macros/Cargo.toml | Enables syn’s visit feature for macro implementation needs. |
| pharmsol-dsl/tests/dsl_authoring_edge_cases.rs | Updates fixtures to name = ... and adds mixed numeric + named output label test. |
| pharmsol-dsl/src/test_fixtures.rs | Updates fixtures to structure = ... and name = ... for recommended authoring style. |
| pharmsol-dsl/src/semantic.rs | Renames analytical kernel→structure and preserves route kind in typed routes. |
| pharmsol-dsl/src/parser.rs | Adds output-statement parsing context to allow numeric output assignment targets; renames kernel→structure. |
| pharmsol-dsl/src/ir.rs | Adds route kind to typed IR and renames analytical field to structure. |
| pharmsol-dsl/src/execution.rs | Adds route kind + declaration_index; changes route buffer sizing and route input load representation. |
| pharmsol-dsl/src/authoring.rs | Renames model→name, kernel→structure; preserves route order + kind; supports numeric output labels. |
| pharmsol-dsl/src/ast.rs | Adds RouteKind to AST and prints analytical blocks with structure = .... |
| examples/two_compartment.rs | Migrates example to declaration-first ode! and metadata-backed route/output lookups. |
| examples/sde_readme.rs | Adds SDE README example using sde! and metadata-backed route/output lookups. |
| examples/one_compartment.rs | Migrates example to analytical! + ode! with metadata-backed route/output indices. |
| examples/ode_readme.rs | Migrates README ODE example to ode! and metadata-backed route/output indices. |
| examples/macro_vs_handwritten_two_cpt.rs | Adds example comparing macro vs handwritten ODE with shared input channel across bolus+infusion. |
| examples/macro_vs_handwritten_one_cpt.rs | Adds example comparing macro vs handwritten ODE on one-compartment IV. |
| examples/dsl_runtime_jit.rs | Updates DSL source to name = .... |
| examples/covariates.rs | Migrates covariates example to declaration-first ode! and metadata-backed route/output indices. |
| examples/compare_solvers.rs | Migrates solver comparison to declaration-first ode! and shared-channel route semantics. |
| examples/analytical_vs_ode.rs | Migrates analytical-vs-ODE comparison to macro surfaces + metadata-backed subject building. |
| examples/analytical_readme.rs | Adds analytical README example using analytical! and metadata-backed route/output indices. |
| README.md | Rewrites quickstart/docs to prefer macro surfaces and documents DSL runtime targets and migrations. |
| .gitignore | Adds docs/ to ignored paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+146
to
+192
| fn resolve_input_label( | ||
| &self, | ||
| label: &ChannelId, | ||
| expected_kind: RouteKind, | ||
| ) -> Result<usize, PharmsolError> { | ||
| if let Some(metadata) = self.metadata() { | ||
| let route = | ||
| metadata | ||
| .route(label.as_str()) | ||
| .ok_or_else(|| PharmsolError::UnknownInputLabel { | ||
| label: label.to_string(), | ||
| })?; | ||
|
|
||
| if route.kind() != expected_kind { | ||
| return Err(PharmsolError::OtherError(format!( | ||
| "input label `{}` is declared as {:?} but used as {:?}", | ||
| label, | ||
| route.kind(), | ||
| expected_kind | ||
| ))); | ||
| } | ||
|
|
||
| return Ok(route.channel_index()); | ||
| } | ||
|
|
||
| label | ||
| .index() | ||
| .ok_or_else(|| PharmsolError::UnknownInputLabel { | ||
| label: label.to_string(), | ||
| }) | ||
| } | ||
|
|
||
| fn resolve_output_label(&self, label: &ChannelId) -> Result<usize, PharmsolError> { | ||
| if let Some(metadata) = self.metadata() { | ||
| return metadata.output_index(label.as_str()).ok_or_else(|| { | ||
| PharmsolError::UnknownOutputLabel { | ||
| label: label.to_string(), | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| label | ||
| .index() | ||
| .ok_or_else(|| PharmsolError::UnknownOutputLabel { | ||
| label: label.to_string(), | ||
| }) | ||
| } |
Contributor
|
Bencher Report| Branch | feat/data-mapping-routes |
| Testbed | mhovd-pgx |
🚨 14 Alerts
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result nanoseconds (ns) (Result Δ%) | Upper Boundary nanoseconds (ns) (Limit %) |
|---|---|---|---|
| Conditional dose modification | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 2,835.20 ns(+134.45%)Baseline: 1,209.29 ns | 1,268.50 ns (223.51%) |
| Create large dataset (100 subjects) | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 94,974.00 ns(+73.25%)Baseline: 54,818.29 ns | 57,190.83 ns (166.07%) |
| Data expand complex (1h intervals) | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 83,597.00 ns(+198.70%)Baseline: 27,987.03 ns | 30,227.27 ns (276.56%) |
| Data expand simple (1h intervals) | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 1,072.20 ns(+118.55%)Baseline: 490.61 ns | 522.06 ns (205.38%) |
| Data expand with additional time | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 121,790.00 ns(+213.43%)Baseline: 38,856.58 ns | 42,055.22 ns (289.60%) |
| Filter exclude subjects | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 39,763.00 ns(+29.18%)Baseline: 30,780.47 ns | 31,492.51 ns (126.26%) |
| Filter include subjects | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 9,664.20 ns(+21.52%)Baseline: 7,952.64 ns | 8,385.26 ns (115.25%) |
| Modify all bolus doses | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 2,845.90 ns(+142.30%)Baseline: 1,174.55 ns | 1,225.93 ns (232.14%) |
| Modify all infusion doses | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 2,895.50 ns(+139.99%)Baseline: 1,206.53 ns | 1,259.57 ns (229.88%) |
| SubjectBuilder multi-occasion | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 532.53 ns(+101.11%)Baseline: 264.79 ns | 275.06 ns (193.60%) |
| SubjectBuilder simple | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 212.71 ns(+103.76%)Baseline: 104.39 ns | 109.22 ns (194.76%) |
| SubjectBuilder with covariates | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 530.38 ns(+91.00%)Baseline: 277.68 ns | 294.64 ns (180.01%) |
| nca_auc_cmax_metrics | 📈 view plot 🚷 view threshold | 582.27 ns(-1.01%)Baseline: 588.21 ns | 616.68 ns (94.42%) |
| nca_population/10 | 📈 view plot 🚷 view threshold | 48,870.00 ns(+3.85%)Baseline: 47,056.63 ns | 50,098.45 ns (97.55%) |
| nca_population/100 | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 130,010.00 ns(+5.97%)Baseline: 122,686.32 ns | 128,111.79 ns (101.48%) |
| nca_population/500 | 📈 view plot 🚷 view threshold | 366,800.00 ns(-8.46%)Baseline: 400,712.11 ns | 436,950.46 ns (83.95%) |
| nca_single_subject | 📈 view plot 🚷 view threshold | 1,005.90 ns(-0.56%)Baseline: 1,011.54 ns | 1,048.96 ns (95.90%) |
| one_compartment | 📈 view plot 🚷 view threshold | 28,872.00 ns(+21.56%)Baseline: 23,751.50 ns | 30,634.38 ns (94.25%) |
| one_compartment_covariates | 📈 view plot 🚷 view threshold | 44,722.00 ns(+40.20%)Baseline: 31,898.39 ns | 47,365.06 ns (94.42%) |
| readme 20 | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 354,380.00 ns(+7.24%)Baseline: 330,467.37 ns | 352,321.45 ns (100.58%) |
| two_compartment | 📈 view plot 🚷 view threshold | 40,016.00 ns(+47.17%)Baseline: 27,190.58 ns | 42,822.20 ns (93.45%) |
…rdering indices, lso the implementation was not complete over the DSL frontend
…or runtime environments.
mhovd
reviewed
May 2, 2026
Comment on lines
+29
to
+31
| // Handwritten closures stay on dense internal channels. | ||
| // Public labels like `iv` and `cp` live in attached metadata, not in | ||
| // the low-level `rateiv[]` / `y[]` buffers. |
Collaborator
There was a problem hiding this comment.
I don't understand what this means
Comment on lines
+32
to
+35
| // Handwritten closures stay on dense internal channels. | ||
| // Public route labels like `load` and `iv` are metadata names; the | ||
| // low-level `bolus[]`, `rateiv[]`, and `y[]` buffers remain indexed by | ||
| // dense internal slots. |
Collaborator
There was a problem hiding this comment.
Same as before, I don't really understand what this means
| } | ||
|
|
||
| #[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] | ||
| pub struct ChannelId(String); |
Collaborator
There was a problem hiding this comment.
Could we add some documentation here? Since it appears to be very central to the definition of events
Comment on lines
+32
to
+35
| // Handwritten closures stay on dense internal slots. | ||
| // Public route labels like `load` and `iv` are metadata names; the | ||
| // low-level `bolus[]`, `rateiv[]`, and `y[]` buffers remain indexed by | ||
| // dense internal slots. |
Comment on lines
+29
to
+31
| // Handwritten closures stay on dense internal slots. | ||
| // Public labels like `iv` and `cp` live in attached metadata, not in | ||
| // the low-level `rateiv[]` / `y[]` buffers. |
| /// * `amount` - Amount of drug administered | ||
| /// * `input` - The compartment number receiving the dose | ||
| pub fn new(time: f64, amount: f64, input: usize, occasion: usize) -> Self { | ||
| /// * `input` - The route label receiving the dose |
Collaborator
There was a problem hiding this comment.
Suggested change
| /// * `input` - The route label receiving the dose | |
| /// * `input` - The input label receiving the dose |
| /// * `time` - Start time of the infusion | ||
| /// * `amount` - Total amount of drug to be administered | ||
| /// * `input` - The compartment number receiving the dose | ||
| /// * `input` - The route label receiving the dose |
Collaborator
There was a problem hiding this comment.
Suggested change
| /// * `input` - The route label receiving the dose | |
| /// * `input` - The input label receiving the dose |
Comment on lines
+97
to
+172
| macro_rules! impl_label_type { | ||
| ($name:ident) => { | ||
| #[derive( | ||
| Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize, | ||
| )] | ||
| pub struct $name(String); | ||
|
|
||
| impl $name { | ||
| pub fn new(label: impl ToString) -> Self { | ||
| Self(label.to_string()) | ||
| } | ||
|
|
||
| pub fn as_str(&self) -> &str { | ||
| &self.0 | ||
| } | ||
|
|
||
| pub fn index(&self) -> Option<usize> { | ||
| self.0.parse::<usize>().ok() | ||
| } | ||
| } | ||
|
|
||
| impl From<String> for $name { | ||
| fn from(value: String) -> Self { | ||
| Self(value) | ||
| } | ||
| } | ||
|
|
||
| impl From<&str> for $name { | ||
| fn from(value: &str) -> Self { | ||
| Self(value.to_string()) | ||
| } | ||
| } | ||
|
|
||
| impl From<usize> for $name { | ||
| fn from(value: usize) -> Self { | ||
| Self(value.to_string()) | ||
| } | ||
| } | ||
|
|
||
| impl AsRef<str> for $name { | ||
| fn as_ref(&self) -> &str { | ||
| self.as_str() | ||
| } | ||
| } | ||
|
|
||
| impl fmt::Display for $name { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| f.write_str(self.as_str()) | ||
| } | ||
| } | ||
|
|
||
| impl PartialEq<usize> for $name { | ||
| fn eq(&self, other: &usize) -> bool { | ||
| self.index() == Some(*other) | ||
| } | ||
| } | ||
|
|
||
| impl PartialEq<$name> for usize { | ||
| fn eq(&self, other: &$name) -> bool { | ||
| other == self | ||
| } | ||
| } | ||
|
|
||
| impl PartialEq<usize> for &$name { | ||
| fn eq(&self, other: &usize) -> bool { | ||
| (**self).eq(other) | ||
| } | ||
| } | ||
|
|
||
| impl PartialEq<&$name> for usize { | ||
| fn eq(&self, other: &&$name) -> bool { | ||
| other.eq(self) | ||
| } | ||
| } | ||
| }; | ||
| } |
Collaborator
There was a problem hiding this comment.
Yes, this macro is shorter than duplicating the code, but it makes reasoning about the structure and features of the labels difficult. Could we refactor it somehow?
Collaborator
There was a problem hiding this comment.
If we purely want to avoid duplicating code, I would prefer a trait-based approach.
| /// Kept as 1-indexed; user must size state arrays accordingly. | ||
| pub fn input(mut self, input: usize) -> Self { | ||
| self.row.input = Some(input); | ||
| /// Preserved as the public route label until model resolution. |
Collaborator
There was a problem hiding this comment.
Suggested change
| /// Preserved as the public route label until model resolution. | |
| /// Preserved as the public input label until model resolution. |
Comment on lines
+387
to
+400
| name = multi_digit_output_runtime | ||
| kind = ode | ||
|
|
||
| params = ke, v | ||
| states = central | ||
| outputs = 2, 10, 11 | ||
|
|
||
| infusion(iv) -> central | ||
|
|
||
| dx(central) = -ke * central | ||
|
|
||
| out(10) = central / v ~ continuous() | ||
| out(2) = central / v ~ continuous() | ||
| out(11) = central / v ~ continuous() |
Collaborator
There was a problem hiding this comment.
This feels a little weird, in contrast to named outputs..
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.
No description provided.