Skip to content

fix(correction): correct integrated areas once instead of every scan - #313

Draft
Jack-Coutts wants to merge 1 commit into
mainfrom
fix/177-correct-integrated-areas-v2
Draft

Jack-Coutts wants to merge 1 commit into
mainfrom
fix/177-correct-integrated-areas-v2

Conversation

@Jack-Coutts

@Jack-Coutts Jack-Coutts commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #177. Fixes #202. Re-applies #213 onto main.

The bug

Natural-isotope correction solved the correction matrix at every scan, set negative scans to 0, and then integrated the corrected traces. In an isotopologue with no real label, noise swings above and below 0. Zeroing each negative scan kept only the positive swings, so unlabelled and small peaks showed false label. A peak with no raw signal at all could export 100% label incorporation.

The tiles had a second problem (#202). For a labelled compound with deconvolution off, the tile verdict read the stored corrected EIC, which does not exist until the first export, so it fell back to raw areas. The export then judged the corrected areas, and a peak near the threshold could pass on screen and fail in the export.

The fix

Every number now integrates the raw traces once and corrects that area vector once. NaturalAbundanceCorrector.correct_areas solves A x = b on the integrated areas and sets negatives to 0. The export, the tile verdicts (get_compound_areas), the Isotopologue ratio bars and Process External Data all use it. The export no longer reads eic_corrected, so the tiles and the export compute the same corrected areas from the same raw EIC.

The corrected EIC plot is unchanged. It still corrects every scan to draw the traces and draws negative scans at 0. That trace is a picture of the correction, not the number: an isotopologue whose corrected area is 0 can still show small positive blips. On the bench data, Glycerol in Std_MM_01 draws M+1 and M+2 blips that the old code integrated to 30.6 and 22.2. The bars, tiles and export now show 0. The reference docs say this.

Docs: the correction reference's section 3 now describes area-first correction and drops the claim that per-scan correction is more accurate. The Process External Data pages drop "Approximate Mode", because both workflows now correct the integrated areas. The README upgrade notes and the export's changelog note say the ratio sheets move for small peaks.

Results

bench/reference.py update is committed with a HISTORY.md entry. Labelled bench dataset:

Sheet Numbers Moved Median change Largest change
Raw Values 14388 0
Corrected Values 14388 1076 15% 100%
Isotope Ratio 14388 1456 5.5% (0.08 points) 100%
% Label Incorporation 3135 735 0.17% (0.04 points) 100%
% Carbons Labelled 3135 1027 0.2% (0.03 points) 100%
Abundances 2665 739 0.1% 100%
  • % Label Incorporation fell in 582 of the 735 cells that moved. 57 cells moved by more than 1 point.
  • The 100% changes are peaks with no raw signal. For example, Fructose in Bio_001 has raw areas of 0 on every isotopologue, and its old corrected M+2 of 0.52 gave 100% label. Now every corrected area is 0 and the label is 0%.
  • One text cell changed. Glycerol's Abundances unit went from nmol to Relative. Its MM-file peaks have no raw signal, so the MRRF that the old per-scan noise produced can no longer be computed.
  • The internal standard (scyllo-Ins) is unchanged on every sheet. The unlabelled dataset is unchanged on every sheet.

No real lab dataset was available on the test machine, so these numbers come from the synthetic bench data only.

Checks

  • New test tests/test_corrected_area_noise_bias.py: an unlabelled M+0 peak of area 3000 with zero-mean noise on M+1 to M+3, deconvolution off and on. The tile areas, the export's corrected areas and the bars must all equal [3000, 0, 0, 0]. On main the tile areas are the raw areas with deconvolution off ([2242.19, 505.44, 213.67, 32.18]) and are biased with deconvolution on ([3004.69, 526.51, 407.73, 399.05]). Both cases pass with the fix.
  • Existing tests that injected a per-scan corrector now inject an area corrector. One export test now uses a formula that cannot be corrected, because a missing stored corrected EIC no longer drops a compound.
  • uv run python -m pytest -q: 673 passed.
  • bench/reference.py check after the update: OK.

Left out

@Jack-Coutts
Jack-Coutts force-pushed the fix/177-correct-integrated-areas-v2 branch from 2fd9df8 to bdd3477 Compare November 10, 2026 19:37
@Jack-Coutts

Copy link
Copy Markdown
Collaborator Author

Review verdict for head bdd3477: not ready. Reviewed by GPT-6.1 Sol. The merge waits for a method decision from the maintainer.

What Sol ran:

  • The full suite: 673 passed. The results check passed against the updated reference.
  • The new tests against origin/main's source. They fail for the expected numerical reasons.
  • Lactate/S1 through all eight combinations of deconvolution on or off, baseline on or off, and legacy or time integration.
  • Tyrosine 218 / Bio_023 and Deoxyribose / Bio_005 on both main and this head, against the same generated database.

What it found:

  1. Blocker: a labelled compound with no formula now exports its raw areas as Corrected Values. On main it is left out. data_provider.py:760-764 returns a copy of the raw areas, and that branch now serves exports with deconvolution off. The repro exported Lactate as [1421.735, 545.402, 188.102, 47.030] under Corrected Values.
  2. Warning: area-first correction moves false label on an unlabelled control. Tyrosine 218 / Bio_023 is marked UNLABELLED_CONTROL by the bench generator. Its % label incorporation rises from 0.885% to 12.758%, and its corrected areas go from about [16705, 111, 38] to [16705, 1933, 510]. Correcting the generator's true areas gives almost all M+0. With baseline correction off, the new result has no labelled area. Per-channel baseline subtraction and clipping are not linear, so "the same except for the per-scan clamp" holds only with baseline off and shared masks.
  3. The PR's claim that every 100% change had no raw signal is wrong. 219 of 242 corrected cells that moved by 100% have raw signal, and so do 27 of 47 label cells. Deoxyribose / Bio_005 goes from 0% to about 100% label. Its raw areas are [0, 0, 0, 132.18] because baseline processing zeroes the first three channels of a real, noisy, co-eluting peak.
  4. Nit: the comment and the reference doc say MATLAB MANIC uses an unconstrained solve followed by a clamp. MATLAB's NaturalIsotopeCorrectionGv3.m uses fmincon with non-negative bounds and a sum-to-one constraint. MATLAB does correct integrated areas, not scans, so this PR moves the order toward MATLAB's, but it does not give numerical parity.

What holds:

  • Tile verdicts use raw areas while export uses corrected areas #202 is fixed. Tiles, the export and the ratio bars agree in all eight combinations for correctable data.
  • The Glycerol switch to Relative is right, because its MM areas integrate to zero. The peak itself is present, though, so describing it as having no signal is wrong.
  • There is no ill-conditioned fallback. A single column and a full time series take the same np.linalg.solve path.

Defects already on main, outside this pull request: corrected bars fall back to raw areas when correction fails, and fitted, baseline-clipped channel vectors can form an envelope that is not physically possible.

Nothing was fixed in review.

@Jack-Coutts
Jack-Coutts marked this pull request as draft November 10, 2026 20:11
@Jack-Coutts

Copy link
Copy Markdown
Collaborator Author

Held by the maintainer's decision. #177 stays open until it can be checked on a real dataset and the baseline handling is decided. #202 will be fixed in a separate small pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tile verdicts use raw areas while export uses corrected areas Natural isotope correction adds false label by clamping every scan

1 participant