fix(chirp): background baseline mean skips NaNs (chirp-edge rows wiped whole columns) - #62
Merged
Merged
Conversation
correct_chirp NaN-fills samples shifted past the time-axis edges — for columns with a negative polynomial shift that lands at the START of the axis, inside typical pre-pump baseline windows. subtract_background's plain per-column mean then turned one NaN row into an all-NaN column, wiping every negative-shift wavelength from the corrected matrix (observed in QPSLab: 409/512 display columns fully null with a background + chirp pipeline). - subtract_background: per-column mean over finite values only; a window with no finite values gets a NaN baseline plus a warning. - _auto_baseline_range: NaN-skipping row means and a NaN-proof argmax so chirp-edge rows cannot blank the onset search. - Tests: chirp+background combo leaves no fully-NaN columns (explicit and auto t_range) and the all-NaN-window warning path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Bug
With a
backgroundcorrection downstream ofcorrect_chirp(QPSLab orders chirp first), every wavelength column with a negative chirp shift came back fully NaN — 409/512 display columns null on a real S1.5 dataset.correct_chirpNaN-fills samples shifted past the time-axis edges. For negative shifts that NaN region is at the start of the axis — exactly inside typical pre-pump baseline windows (e.g. −20…0 ps).subtract_backgroundcomputed the per-column baseline with a plainmean, so a single NaN row poisoned the entire column viadata .- baseline'. Positive-shift columns survived only because their NaN row is at the end of the axis, outside the window.Fix
subtract_background(::TimeResolvedMatrix): baseline is now the per-column mean over finite values only. A window with no finite values yields a NaN baseline (the column stays NaN — no background is determinable) and emits a warning._auto_baseline_range: row means skip NaNs, and NaN gradients are scored-Infso they cannot win the onsetargmax(previously a NaN would, sinceislesssorts NaN last).Tests
Full suite: 1899/1899 pass locally.
QPSLab will pin
[compat] OpticalSpectroscopy = "0.1.3"and add an end-to-end regression test on its side.🤖 Generated with Claude Code