feat: Rework trait definitions#277
Open
Siel wants to merge 8 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the simulation trait hierarchy to cleanly separate solver mechanics from user-facing simulation/likelihood APIs, replacing the old Equation* stack with a new core module (Solver, ModelInfo, Caching, Simulate) and moving handwritten backends under simulator/backends.
Changes:
- Introduces
src/core/traits + sharedModelCore, and re-exports the new public surface fromlib.rs. - Migrates ODE/Analytical/SDE backends to the new traits and relocates the backend modules (
simulator/equation→simulator/backends). - Updates likelihood + optimization utilities and rewrites tests/examples/benches to use
backends::*+Simulate.
Reviewed changes
Copilot reviewed 50 out of 53 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_solvers.rs | Update test model construction to use backends::* and new metadata path. |
| tests/test_pf.rs | Migrate SDE test to backends::* and ModelKind relocation. |
| tests/support/runtime_corpus.rs | Update runtime corpus to Simulate + backends::* imports and metadata paths. |
| tests/support/bimodal_ke.rs | Update ODE reference model to backends::* and pharmsol::metadata. |
| tests/sde_macro_lowering.rs | Update SDE macro lowering tests to new backend/module paths. |
| tests/ode_optimizations.rs | Update ODE/Analytical optimization-parity tests to new backend/module paths. |
| tests/ode_macro_lowering.rs | Update ODE macro lowering tests to backends::* + new metadata path. |
| tests/numerical_stability.rs | Update numerical stability tests for moved backends/metadata. |
| tests/full_feature_macro_parity.rs | Update parity tests for macro vs handwritten models with new module layout. |
| tests/authoring_parity_corpus.rs | Update authoring parity corpus tests to backends::* and new metadata location. |
| tests/analytical_macro_lowering.rs | Update analytical macro lowering tests to backends::* and new metadata location. |
| src/simulator/mod.rs | Rename public module equation → backends and adjust docs/examples accordingly. |
| src/simulator/likelihood/subject.rs | Add PredictionsContainer impl for SubjectPredictions for the new Simulate pipeline. |
| src/simulator/likelihood/mod.rs | Switch batch/subject likelihood helpers from Equation to Simulate + slice-based predictions access. |
| src/simulator/likelihood/matrix.rs | Switch likelihood matrix computation from Equation to Simulate::log_likelihood. |
| src/simulator/equation/mod.rs | Remove legacy Equation trait stack and related types (superseded by core). |
| src/simulator/cache.rs | Update doc links to reflect new backend module location. |
| src/simulator/backends/sde/em.rs | Add new Euler–Maruyama solver implementation for SDE backend infrastructure. |
| src/simulator/backends/ode/mod.rs | Refactor handwritten ODE backend to implement new core traits + shared ModelCore. |
| src/simulator/backends/ode/closure.rs | Add revised diffsol closure-backed ODE problem implementation (infusion schedule, RHS, etc.). |
| src/simulator/backends/mod.rs | New backend module root + re-exports and cache-key hashing. |
| src/simulator/backends/analytical/two_compartment_models.rs | Update analytical-model tests to use simulator/backends::* and Simulate. |
| src/simulator/backends/analytical/two_compartment_cl_models.rs | Update analytical-model tests to use new backend paths and Simulate. |
| src/simulator/backends/analytical/three_compartment_models.rs | Update analytical-model tests to use new backend paths and Simulate. |
| src/simulator/backends/analytical/three_compartment_cl_models.rs | Update analytical-model tests to use new backend paths and Simulate. |
| src/simulator/backends/analytical/one_compartment_models.rs | Update analytical-model tests to use new backend paths and Simulate. |
| src/simulator/backends/analytical/one_compartment_cl_models.rs | Update analytical-model tests to use new backend paths and Simulate. |
| src/simulator/backends/analytical/mod.rs | Refactor handwritten Analytical backend to new core traits + ModelCore and standard loop. |
| src/parameters.rs | Update tests/imports to use core::Simulate instead of Equation. |
| src/parameter_order.rs | Point metadata import at core::metadata. |
| src/optimize/parameters.rs | Switch optimizer generic bound from Equation to Simulate and update docs. |
| src/optimize/mod.rs | Update module docs for the new simulation trait surface. |
| src/optimize/effect.rs | Add/adjust module-level documentation. |
| src/lib.rs | Re-export core traits/metadata, move backend exports to simulator/backends, adjust prelude and macros. |
| src/dsl/native.rs | Port native runtime models to implement Solver/ModelInfo/Caching/Simulate and update metadata binding. |
| src/dsl/jit.rs | Update JIT tests to new backend paths and trait names. |
| src/data/error_model.rs | Remove AssayErrorModels::bind_to(Equation) (binding now happens via core helper). |
| src/core/state.rs | New State trait extracted into core. |
| src/core/solver.rs | New Solver trait defining state advancement + observation processing + batch toggle. |
| src/core/simulate.rs | New Simulate trait + standard_event_loop + error model binding + parameter hashing. |
| src/core/predictions.rs | New Predictions trait (ported from legacy equation module). |
| src/core/model_info.rs | New ModelInfo trait with metadata-driven label resolution and event lowering. |
| src/core/model_core.rs | New shared ModelCore storing dimensions, metadata, and caches. |
| src/core/mod.rs | New core module root re-exporting all core traits and helpers. |
| src/core/caching.rs | New Caching trait encapsulating caches and cache control operations. |
| pharmsol-macros/src/lib.rs | Update macro output to reference new metadata/backends paths and expand macro docs. |
| examples/macro_vs_handwritten_two_cpt.rs | Update example to use backends::* and new metadata location. |
| examples/macro_vs_handwritten_one_cpt.rs | Update example to use backends::* and new metadata location. |
| examples/compare_solvers.rs | Update example to use backends::* type paths. |
| benches/native_matrix.rs | Update benchmark to new cache API (without_cache) and remove old Cache trait usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+3
to
+7
| //! This module provides a [`ParameterOptimizer`] that refines a single parameter | ||
| //! Given an [`Equation`], observed [`Data`], and [`AssayErrorModels`] via | ||
| //! Nelder‑Mead optimization in log‑space. The optimizer finds the parameter vector | ||
| //! that minimizes the negative log-likelihood of the model predictions against the data, | ||
| //! as measured by the provided error models. |
| use crate::simulator::backends::ode::{ExplicitRkTableau, OdeSolver}; | ||
| use crate::simulator::backends::analytical::one_compartment_with_absorption; | ||
| use crate::core::Simulate; | ||
| use crate::core::Predictions as PredictionTrait; |
Contributor
|
| Branch | feature/traits_rework |
| 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.
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.
This untangles two concerns that were awkwardly mashed together: "what can simulate" vs. "what can solve." The old
Equation/EquationPriv/EquationTypes/EqnKindstack tried to cover both at once, which meant adding a new backend always came with copy-paste and fighting the trait hierarchy.The replacement is four traits living in a new
coremodule, each with one job:Solver— how state moves through time. You must implementsolve(),process_observation(), andinitial_state(). Batch solvers (like diffsol) also setis_batch()totrueand manage events themselves.ModelInfo— what the model looks like. You must implementlag()andfa()(both return the backend's own closures).nstates(),ndrugs(),nout(), andmetadata()are free.Caching— where prediction and error-model caches live. You must implement nothing. All five methods (prediction_cache(),error_model_cache(),with_cache_capacity(),without_cache(),clear_cache()) are free.Simulate— the user-facing API. You must implementsimulate_subject()(for standard per-event backends that's juststandard_event_loop::<Self, _>(self, ...)) andkind().predictions(),log_likelihood(), andestimate_predictions()are free default methods on the trait.There's also a shared
ModelCore<C>struct that owns dimensions, metadata, and both caches soODE,Analytical, andSDEdon't each reimplement the same builder and invalidation dance.State,Predictions, and the metadata module moved intocore/alongside everything else.simulator/equation/becamesimulator/backends/.This is how adding a new solver loos like:
On my tests there are no regressions in performance.