feat: add NormalizedRow API, ResidualErrorModel and NCA #187

Siel · 2025-12-30T14:33:30Z

This commit adds two related features:

NormalizedRow API (parser/)

New struct for format-agnostic data parsing
Decouples column mapping from event creation logic
Full ADDL/II expansion support (both positive and negative directions)
Refactors pmetrics.rs to use NormalizedRow internally
Enables external tools (like vial) to reuse parsing logic without reimplementing ADDL expansion

ResidualErrorModel (data/)

New for parametric algorithms (SAEM, FOCE)
Uses prediction-based sigma (vs observation-based in ErrorModel)
Adds and functions
Documentation clarifying ErrorModel vs ResidualErrorModel usage

Both features are independent but included together to avoid merge conflicts.

This commit adds two related features: ## NormalizedRow API (parser/) - New struct for format-agnostic data parsing - Decouples column mapping from event creation logic - Full ADDL/II expansion support (both positive and negative directions) - Refactors pmetrics.rs to use NormalizedRow internally - Enables external tools (like vial) to reuse parsing logic without reimplementing ADDL expansion ## ResidualErrorModel (data/) - New for parametric algorithms (SAEM, FOCE) - Uses prediction-based sigma (vs observation-based in ErrorModel) - Adds and functions - Documentation clarifying ErrorModel vs ResidualErrorModel usage Both features are independent but included together to avoid merge conflicts.

Copilot

Pull request overview

This PR adds two independent features to support different algorithmic approaches in pharmacometric modeling:

Purpose: Enable external tools to reuse pharmsol's data parsing logic and provide prediction-based error models for parametric estimation algorithms.

Key Changes:

Introduces NormalizedRow API for format-agnostic data parsing with full ADDL/II expansion (positive and negative directions)
Adds ResidualErrorModel for parametric algorithms (SAEM, FOCE) that uses prediction-based sigma calculations
Refactors pmetrics.rs to use NormalizedRow internally
Adds log_likelihood_batch and log_likelihood_subject helper functions for batch likelihood computation

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/data/residual_error.rs	New module implementing ResidualErrorModel and ResidualErrorModels for parametric algorithms with prediction-based sigma computation
src/data/parser/normalized.rs	New module providing NormalizedRow/NormalizedRowBuilder for format-agnostic data parsing with ADDL expansion
src/data/parser/pmetrics.rs	Refactored to use NormalizedRow internally, removing duplicate ADDL logic
src/simulator/likelihood/mod.rs	Adds log_likelihood_batch and log_likelihood_subject functions; updates documentation to clarify ErrorModel vs ResidualErrorModel usage
src/simulator/equation/mod.rs	Documentation update clarifying when to use ResidualErrorModels
src/simulator/equation/sde/mod.rs	Removes redundant comment about underflow
src/optimize/effect.rs	Adds comprehensive documentation for get_e2 function
src/lib.rs	Exports new types through prelude
src/data/parser/mod.rs	Adds normalized module
src/data/mod.rs	Adds residual_error module and documentation
src/data/error_model.rs	Adds utility methods is_proportional/is_additive and clarifies observation-based sigma usage

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-30T14:39:26Z

src/simulator/likelihood/mod.rs

+/// # Example
+/// ```ignore
+/// use pharmsol::{log_likelihood_batch, ResidualErrorModel, ResidualErrorModels};
+///
+/// let residual_error = ResidualErrorModels::new()
+///     .add(0, ResidualErrorModel::constant(0.5));
+///
+/// let log_liks = log_likelihood_batch(
+///     &equation,
+///     &data,
+///     &parameters,
+///     &residual_error,
+/// )?;
+/// ```


The documentation example cannot compile because log_likelihood_batch and log_likelihood_subject are not re-exported at the crate root. They are only available in the prelude under pharmsol::prelude::simulator. The import should be either: use pharmsol::prelude::simulator::{log_likelihood_batch, log_likelihood_subject}; or the functions should be added to the crate root re-exports. Additionally, ResidualErrorModel and ResidualErrorModels would need to be imported from pharmsol::prelude::data or directly from pharmsol (which works via the pub use crate::data::*; re-export).

Copilot · 2025-12-30T14:39:26Z

src/data/parser/normalized.rs

+
+                // Handle ADDL/II expansion
+                if let (Some(addl), Some(ii)) = (self.addl, self.ii) {
+                    if addl != 0 && ii > 0.0 {


The ADDL expansion logic requires ii > 0.0, meaning negative or zero interdose intervals are silently ignored. This could lead to unexpected behavior where users provide negative II values and no additional doses are generated. Consider either: (1) documenting this requirement explicitly in the function documentation, or (2) returning an error when ADDL is non-zero but II is not positive, to make the failure explicit rather than silent.

ii Should always be >0.0

* nca * wip: current version * feat: nca * clenup * chore: documentation * chore: cleanup * chore: cleanup * chore: deprecating ErrorModel in favor of AssayErrorModel, subdividing the likelihood module and deprecating linear space likelihood calculation functions * feat: the Data parsing is centraliced to NormalizedRow * feat: the ErrorModel -> AssayErrorModel * feat: validation * chore: cleanup * chore: cleanup

mhovd · 2026-01-13T14:00:56Z

tests/nca/test_terminal.rs

+        9.07,  // 100 * e^(-0.1*24)
+    ];
+
+    let result = calculate_lambda_z_adjusted_r2(&times, &concs, None);


Does this function exist?

That is the old API for that function, the weird thing is that tests are passing. It seems those might not be wired up. I'll look into that

I just pushed a new commit with the fix

mhovd · 2026-01-14T08:45:06Z

src/data/error_model.rs

+///
+/// This type alias is provided for backward compatibility.
+/// New code should use [`AssayErrorModel`] directly.
+#[deprecated(


Why not just remove it, since we are going for a new minor verison anyway?

I ended up deprecating all the old functions to make clear what I am removing from the API to make it easy to find / discuss.

I am aiming to a major patch, the API is changing a lot, there are other deprecated functions

src/optimize/effect.rs

Co-authored-by: Markus Hovd <markushh@uio.no>

mhovd · 2026-01-14T09:03:02Z

src/data/parser/normalized.rs

+//!
+//! This module provides a format-agnostic intermediate representation that decouples
+//! column naming/mapping from event creation logic. Any data source (CSV with custom
+//! columns, Excel, DataFrames) can construct [`NormalizedRow`] instances, then use
+//! [`NormalizedRow::into_events()`] to get properly parsed pharmsol Events.
+//!
+//! # Design Philosophy
+//!
+//! The key insight is separating two concerns:
+//! 1. **Row Normalization** - Transform arbitrary input formats into a standard representation
+//! 2. **Event Creation** - Convert normalized rows into pharmsol Events (with ADDL expansion, etc.)
+//!
+//! This allows any consumer (GUI applications, scripts, other tools) to bring their own
+//! "column mapping" while reusing parsing logic.


Suggested change

//!

//! This module provides a format-agnostic intermediate representation that decouples

//! column naming/mapping from event creation logic. Any data source (CSV with custom

//! columns, Excel, DataFrames) can construct [`NormalizedRow`] instances, then use

//! [`NormalizedRow::into_events()`] to get properly parsed pharmsol Events.

//!

//! # Design Philosophy

//!

//! The key insight is separating two concerns:

//! 1. **Row Normalization** - Transform arbitrary input formats into a standard representation

//! 2. **Event Creation** - Convert normalized rows into pharmsol Events (with ADDL expansion, etc.)

//!

//! This allows any consumer (GUI applications, scripts, other tools) to bring their own

//! "column mapping" while reusing parsing logic.

mhovd · 2026-01-14T09:08:29Z

src/data/parser/normalized.rs

+            Event::Observation(obs) => {
+                assert_eq!(obs.time(), 1.0);
+                assert_eq!(obs.value(), Some(25.5));
+                assert_eq!(obs.outeq(), 0); // Converted to 0-indexed


I am not sure if we want to convert this to 0-indexed if we are parsing a "NormalizedRow". I think that is something that is special to Pmetrics, because R is 1-indexed. So in this case, it should be 1.

I think we should stop removing one to the index values all around the codebase. If pmetrics wants to use compartment 1 we can do that by allocating two elements in the vector. keeping track if it is 0-index or 1-index has been very painful lately for me

I love what I am reading! Yes please :) :) :) :) :)

Copilot AI review requested due to automatic review settings December 30, 2025 14:33

Copilot started reviewing on behalf of Siel December 30, 2025 14:33 View session

Siel force-pushed the feat/normalized-row-api branch from e418fad to da044a9 Compare December 30, 2025 14:37

Copilot AI reviewed Dec 30, 2025

View reviewed changes

Siel changed the title ~~feat: add NormalizedRow API and ResidualErrorModel~~ feat: add NormalizedRow API and ResidualErrorModel and NCA Jan 11, 2026

Siel changed the title ~~feat: add NormalizedRow API and ResidualErrorModel and NCA~~ feat: add NormalizedRow API, ResidualErrorModel and NCA Jan 11, 2026

Siel added 4 commits January 11, 2026 19:35

Merge branch 'main' into feat/normalized-row-api

4d3109c

chore: cleanup

1e0c4ed

chore: fmt

f24d525

feat: more validation scenarios

0229425

mhovd reviewed Jan 13, 2026

View reviewed changes

Siel added 2 commits January 13, 2026 15:33

chore: wire up the tests, and update them to the newer API

eb99b43

chore: fmt

49e5fc6

mhovd reviewed Jan 14, 2026

View reviewed changes

src/optimize/effect.rs Outdated Show resolved Hide resolved

Update src/optimize/effect.rs

8f4b155

Co-authored-by: Markus Hovd <markushh@uio.no>

mhovd reviewed Jan 14, 2026

View reviewed changes

feat: add NormalizedRow API, ResidualErrorModel and NCA #187

Are you sure you want to change the base?

feat: add NormalizedRow API, ResidualErrorModel and NCA #187

Uh oh!

Conversation

Siel commented Dec 30, 2025

NormalizedRow API (parser/)

ResidualErrorModel (data/)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants