Fix CVM inference path (restore branch, reference table, tag collision)

[Hahyun-Lee]

Summary

SPARE-CVM inference does not run on main. This PR restores the CVM branch and fixes three defects that stopped the CVM models even where the branch exists. Related to #8, which observes that inference only handles RG and CL.

Verified end-to-end on 20 subjects × 5 CVM models (T2D, HYPERTENSION, OBESITY, SMOKING, HYPERLIPIDEMIA). All five produce output, and values match a manually patched run 100/100.

What was wrong

I hit these while running the published cbica/nichart_spare_score_runall:0.0.4 image and then reproducing against main.

1. The CVM inference branch is missing from main.
infer_svm_model() handles RG and CL only, so -t CVM falls through every branch and X is unbound. The branch does exist at 38cfe719 — the commit the published 0.0.4 image is built from — and main is 6 commits ahead of it, so this looks like it was dropped along the way rather than never written. This PR restores it.

2. The residualization reference table is not the one that ships.
apply_cvm_residualization() reads reference/covparams_scaler_sparecvms_dl2.csv, but the repo ships covparams_scaler_sparecvms_dl.csv:

Error: [Errno 2] No such file or directory: '.../reference/covparams_scaler_sparecvms_dl2.csv'

The patch prefers _dl2 when present and otherwise falls back to the packaged _dl, raising an explicit error naming both if neither exists. If _dl2 is meant to be a different table, shipping it would be the better fix — please say so and I'll switch this to a plain filename correction.

3. The ICV row of the reference table is looked up under the wrong name.
The input frame is renamed DL_MUSE_Volume_702 → DLICV, but the reference table still lists that row as DL_MUSE_Volume_702. The lookup returns an empty array:

Error: operands could not be broadcast together with shapes (20,) (0,)

The commented-out df_params.rename(columns={'DLICV': dlicv_col}) right below the read suggests this was known. The patch maps the Features value instead of the column, which is where the mismatch actually is.

4. Age/Sex/ICV are dropped before the CVM path needs them.
The column subset keeps only key_variable + feature_names, but apply_cvm_residualization() is called after it and requires Age, Sex, and the ICV column:

Error: 'Age'

The patch keeps those three for CVM-family spare_types only, so RG/CL/AD behaviour is unchanged.

5. All five CVM models collapse to one tag.
extract_spare_tag() matches SPARE-([A-Za-z0-9]+)-, which returns CVM for every SPARE-CVM-<CONDITION>-Harmonized.joblib. Four of the five then hit the duplicate-tag check in run_all.py and are skipped with only Skipping due to duplicate spare tag... in the log — no error, exit code 0. The patch keeps the condition (CVM-T2D, CVM-HYPERTENSION, …) so each model gets its own output column.

Verification

Run on an aarch64 host (DGX Spark GB10), scikit-learn 1.9.0, against this branch with the reference directory left exactly as the repo ships it:

model	-t	result
SPARE-CVM-HYPERLIPIDEMIA-Harmonized	CVM	20/20 subjects
SPARE-CVM-T2D-Harmonized	CVM	20/20 subjects
SPARE-CVM-HYPERTENSION-Harmonized	CL	20/20 subjects
SPARE-CVM-OBESITY-Harmonized	CL	20/20 subjects
SPARE-CVM-SMOKING-Harmonized	CL	20/20 subjects

Tag uniqueness after the change: 7/7 distinct across the five CVM models plus SPARE-BA-RAW-* and SPARE-AD-Harmonized-* (previously 3/7).

RG/CL/AD paths are untouched apart from the subset guard, which is gated on spare_type.

Notes / open questions

• -t has to match the model's meta_data['spare_type'] — CVM for T2D and HYPERLIPIDEMIA, CL for the other three. That is not obvious from the filenames; happy to add a note to the README if useful.
• Input needs a Sex_M one-hot column for the CL CVM models. apply_cvm_residualization() derives it when missing, but the CL path does not go through that function, so those three fail with Missing columns:Sex_M. Left alone here since it may be intended, but flagging it.
• This does not touch training, where the CVM task is still commented out (CVM models implementation #8).

[Copilot]

Pull request overview

Restores and fixes the CVM-family inference path so CVM models can run on main, including residualization reference-table handling and improved model tagging in run_all to avoid collisions.

Changes:

• Add CVM-family inference branch in infer_svm_model() and preserve Age/Sex/ICV through subsetting for residualization.
• Make apply_cvm_residualization() robust to the shipped reference filename (*_dl.csv) while preferring *_dl2.csv when present, and fix ICV feature-name mismatch.
• Improve run_all.py tag extraction for CVM models to avoid duplicate-tag skipping.

Reviewed changes

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

File	Description
scripts/run_all.py	Adjusts tag extraction to distinguish CVM condition models.
NiChart_SPARE/svm.py	Restores CVM-family inference path and preserves demographics/ICV for residualization.
NiChart_SPARE/data_prep.py	Fixes CVM residualization reference-file selection and aligns ICV feature naming with the table.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Hahyun-Lee]

Both comments were correct — fixed in 8b4d79a, and chasing them surfaced a pre-existing crash as well.

1. rename_spare_columns() didn't cover SPARE_CVM — confirmed. With the tag fix, all five CVM models now run, but the two whose spare_type is CVM (T2D, HYPERLIPIDEMIA) both emitted SPARE_CVM and collided as _x/_y on merge. Rather than adding SPARE_CVM to the enumeration, the function now renames any SPARE_<mode> column to SPARE_<tag>, so this can't recur when another inference mode is added. GT_BA is carried alongside GT_RG for the same reason.

2. Up-front validation for the CVM residualization columns — done. Age/Sex/ICV are appended to the required-column list checked before subsetting, so a missing one now reports Error: Missing columns:Age like any other required column instead of a KeyError from inside apply_cvm_residualization().

3. Pre-existing: run_all.py crashes before model selection on current main.

Verifying the two comments above meant running run_all.py itself, which I hadn't done in the first round — I had only exercised NiChart_SPARE directly, per model. That's how the column collision escaped me, and it also hid this:

Traceback (most recent call last):
  File "scripts/run_all.py", line 151, in <module>
    if not dlicv_found:
NameError: name 'dlicv_found' is not defined

dlicv_found is referenced but never assigned anywhere in the file — the block that set it appears to have been commented out in 3583051 while the reference stayed. This fires for every --category, not just cvm, so run_all.py on main currently cannot complete any run. It's assigned from the DLICV column check it was evidently meant to report on. The adjacent print was also missing its f prefix and emitted the literal {df_original.columns}.

Verification — this time end-to-end through run_all.py, 20 subjects, aarch64, scikit-learn 1.9.0, reference directory left exactly as the repo ships it:

run	rc	models	duplicate-tag skips	_x/_y columns
--category cvm --harmonize	0	5/5	0	0
--category misc --harmonize	0	2/2	0	0

CVM output columns: SPARE_CVM-T2D, SPARE_CVM-HYPERTENSION (+_decision_function), SPARE_CVM-OBESITY (+df), SPARE_CVM-HYPERLIPIDEMIA, SPARE_CVM-SMOKING (+df).
misc: SPARE_AD, SPARE_AD_decision_function, SPARE_BA, BA_GT_RG.

Values are identical to per-model direct invocation — CVM 100/100 cells, SPARE-BA 20/20, SPARE-AD 20/20 — so the tag/rename change doesn't alter any prediction.

One open question from the original description still stands: if covparams_scaler_sparecvms_dl2.csv is meant to be a different table from the shipped _dl.csv, shipping it would be the better fix and I'll reduce that hunk to a plain filename correction.