fix(alignment): correct gauss_model midpoint anchor (GH #34) - #35
Merged
Conversation
gauss_model and joint_gauss_model reconstructed sampled curves by passing the augmented level coordinate to srsf_inverse as f0 (the value at argvals[0]). But build_augmented_srsfs encodes the curve level at the domain MIDPOINT (m/2), so every generated curve was shifted by a constant equal to its midpoint level (~+1 for the reporter's peak-1 bumps), and the sample mean did not match the data mean. Fix: reconstruct from a zero start, then shift the curve so its midpoint equals the recovered level. Added a regression test asserting the sample-mean offset from the data mean stays near zero (fails at the old ~+1.0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #35 +/- ##
=======================================
Coverage 89.01% 89.02%
=======================================
Files 200 200
Lines 44099 44131 +32
=======================================
+ Hits 39255 39287 +32
Misses 4844 4844
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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.
Fixes #34.
Problem
gauss_model(andjoint_gauss_model) produced samples whose mean was offset by a constant (~+1.0) from the data mean, and whose curves left the data range.Root cause
build_augmented_srsfs(elastic_fpca.rs:717) encodes the curve level at the domain midpointm/2:But the generative reconstruction recovered that value and passed it to
srsf_inverseasf0, whichsrsf_inversetreats asf(argvals[0])— the start. So each reconstructed curve was pinned at its start to the midpoint level, shifting the whole curve by a constant (≈ the midpoint value, ~1.0 for the reporter's peak‑1 bumps).Fix
Reconstruct from a zero start and shift so the midpoint matches the recovered level (
generative.rs, bothgauss_modelandjoint_gauss_model):Tests
New regression
gauss_model_sample_mean_no_constant_offsetasserts the average sample‑mean offset from the data mean stays < 0.3 (it was ~+1.0). Full alignment (244) + elastic_fpca (15) + R‑validation (173) suites pass; clippy clean.Note (not changed here)
The same midpoint‑vs‑start anchor mismatch also exists in the
elastic_fpca.rseigenfunctions_freconstruction (two sites). Those feed a display output (the generative samples useeigenfunctions_q, not_f) and are R‑validated, so I've left them for a separate, carefully‑validated change rather than bundle them here.🤖 Generated with Claude Code