Audit and hardening: 35 findings, 58 regression tests, published behaviour preserved by flag

[snuconnectome]

Audit of this repository at eed038c ("Communications Biology ver.") and a hardening patch.
35 findings — 7 that prevent the released code from running at all, 9 that change reported
numbers or their interpretation — with a 58-test regression suite and 20 new flags. Full
detail, with the measurement behind each claim, in
docs/audit/AUDIT.md.

15 of the 35 were established by importing the released modules unmodified and exercising them,
not by reading them. No cohort data was available to this audit, so nothing here is a claim
about the paper's reported accuracy, and no fix is claimed to improve AUROC.

Three findings that bear on published claims

1. The interpreted attention maps carry no label information.
Transformer_Finetune_Three_Channels computes three band-specific ROI×ROI spatial attention
maps and returns them, but they never enter the prediction:

d(prediction) / d(high_spatial_attention.qkv.weight)     = 0.0   (exactly)
d(prediction) / d(low_spatial_attention.qkv.weight)      = 0.0
d(prediction) / d(ultralow_spatial_attention.qkv.weight) = 0.0

while the band-repulsion loss delivers |∇| ≈ 2817 to the same parameters. visualization.py
then computes its saliency by backpropagating that same repulsion loss. For S a sum of
mean-L1 distances, d(-log S)/dh_ij = -[sign(h-l)+sign(h-u)]/(numel·S) — five discrete values
times one global scalar; measured on a (2,8,400,400) map, 4 of the 5 possible values across
2.56M entries, 33.4% exactly 0. The diagnosis enters only through which subjects are averaged.

The step-1 vanilla_BERT baseline does wire its spatial branch into the prediction (|∇| 406),
so a step-1-vs-step-2 comparison changes the head wiring as well as the frequency decomposition.

Fix: --spatial_head puts the maps on the prediction path; --attribution label_gradient
(new default) attributes the model's own decision and refuses to run when the maps are
disconnected; --n_permutations adds a label-permutation null with FWE correction.
--attribution spatial_difference reproduces the published maps.

2. Reported operating-point metrics are prevalence-determined constants.
LossWriter is constructed with Trainer.val_threshold (= 0) and nothing updates it, so
Metrics.ROC_CURVE(name='test') thresholds sigmoid outputs at 0. Reproduced through the
released Metrics on scores with AUROC 0.933, prevalence 0.275:

	threshold	sens.	spec.	bal. acc.	F1
validation	0.506	0.900	0.866	0.878	0.794
test, as released	0.000	1.000	0.000	0.500	0.431
test, val threshold carried	0.506	0.891	0.866	0.878	0.794

At threshold 0, F1 is pinned at 2p/(1+p) and balanced accuracy at 0.500 regardless of the
model. AUROC is unaffected. Fix: the validation operating point is stored and carried over,
with a guard that warns when the test threshold falls outside (0,1).

3. The band-repulsion objective is optimised by hub attention.
Rows of each map are softmax probability vectors, so each pairwise mean-L1 is ≤ 2/N and
S ≤ 6/N. At N=400 that ceiling is 0.015 and it is attained (measured 0.0150) exactly when
every row is one-hot and the three bands point at different targets — the global optimum of
-log S is star/hub attention, independently of the data. S = 0 when the maps coincide exactly, so -log S
is singular there and its gradient scales as 1/S; random initialisation is not that point but
is close to it (measured S = 0.00278, finite loss 588.6 at λ=100), and |∇|∞ reaches 0.50 at
logit scale 1e-4 versus 1.6e-3 at scale 1. 400 steps of the repulsion term alone, at the
released learning rate on the released Attention, moved row entropy from 0.9908 to 0.9625 of
log N and peak row weight from 2.55/N to 6.11/N — a 2.39x increase.

Fix: --spat_diff_loss_type {minus_log (published, verbatim), minus_log_eps, neg_linear, cosine}, --spat_diff_entropy_weight to penalise hub collapse, --spatial_loss_warmup to ramp
λ past the initial transient.

Blocking bugs — the released code does not run as shipped

model.py	init_weights() is called before self.bert and self.cls_embedding exist, so it initialises nothing; it is also not the documented entry point, and on transformers ≥ 5 construction raises AttributeError: no attribute 'all_tied_weights_keys'. Now post_init() after construction.
dataloaders.py	np.in1d was removed in NumPy 2.0 → np.isin.
trainer.py	torch.cuda.nvtx calls are fatal on a CPU-only build.
model.py	Attention.forward references self.proj / self.proj_drop, never created.
model.py	head-count selection has no else, so any sequence length divisible by neither 12 nor 8 (e.g. ABIDE's native 280) raises UnboundLocalError.
learning_rate.py	--lr_warmup is absolute but T_0 is 30 % of total steps; any run under lr_warmup/0.3 steps trips a message-less assert.
trainer.py	wandb was a hard dependency even offline.

Evaluation protocol

• drop_last=True + RandomSampler on the evaluation loaders. At this repository's own
  ABIDE N=141, the released 70/15/15 split gives train 98 / val 21 / test 22, so
  finetune_MBBN.slurm's --batch_size_phase2 16 discarded 6 of 22 test subjects (27.3 %)
  and 5 of 21 validation subjects at every evaluation — a different 6 and 5 each epoch. At
  batch 32 the fold yields zero batches.
• pretrain_MBBN.slurm passed --sequence_length_phase4 464 while running step 3.
  sort_args keeps only *_phase3 keys for step 3, so the flag was silently discarded,
  pretraining ran at the phase-3 default 348, and loading that checkpoint into the 464-wide
  fine-tuning model raises RuntimeError on 4 of 44 shared keys. Corrected to
  --sequence_length_phase3; sort_args now warns when an explicitly-passed phase-tagged
  option is dropped.
• Splits ignored family structure and site, and the split seed was the weight-init seed.
  Added --group_by_family, --site_stratify, --leave_one_site_out, --split_seed.
  Note: 17 ABIDE sites × 2 classes = 34 strata cannot fit a 22-subject test fold, so joint
  stratification degrades to target-only with a warning — site cannot be balanced at this
  cohort size, and --leave_one_site_out or k-fold is the honest alternative.
• Selection is now validation-only; the held-out fold is scored once at the end with the
  validation operating point (--eval_test_every_epoch restores the published behaviour).

Training safety, robustness, cost

• Prediction head Linear → BatchNorm1d(1) → Dropout(0.6) on the scalar logit: one
  subject's logit moves SD 0.293 (range 1.98, sigmoid p spanning 0.107–0.464) from its
  batch-mates alone; a plain linear head gives SD exactly 0. Dropout(0.6) sets 61.4 % of
  training logits to exactly 0. --head_type linear is the new default, and the same head is
  now shared by the baseline and MBBN so the architecture comparison is unconfounded.
• NaN losses were replaced by 0.0 and training continued — a diverged run looked converged.
  --nan_policy raise is the new default.
• Masked pretraining used one deterministic mask for every subject and every epoch, hiding
  380/400 ROIs plus every other window. --random_mask resamples per subject per epoch, fills
  with a learned mask token rather than 0, and --mask_loss_on_masked_only scores only hidden
  positions.
• attention_mask=None, so CLS pooling averaged over zero padding; ABIDE was padded to a
  hardcoded 464 with an off-by-one for odd pad. Now a real padding mask, symmetric padding, and
  padding applied only when a pretrained checkpoint requires it.
• The three bands passed through the same encoder with no band identity → --band_embedding.
• Gradient accumulation did not accumulate (zero_grad every step); --clip_max_norm was
  declared but never read; torch.compile results were discarded; run_phase returned a
  checkpoint filename save_checkpoint_ never writes; every improving epoch wrote a new
  ~325 MB checkpoint.
• --cache_bands memoises the per-subject knee fit (427× on ABIDE). Opt-in: the per-subject
  ratio is large but the absolute saving is modest — minutes for ABIDE fine-tuning, under an
  hour for ABCD, and 22.8 wall-hours for the 1000-epoch Schaefer-400 UKB pretraining run at a
  181 GB cache.

Reproducibility

Defaults reproduce released behaviour except where that behaviour is itself the defect (the
test operating point, the dropped evaluation subjects, the mis-tagged pretraining flag, the
seven blocking bugs). Published forms are kept verbatim rather than reimplemented:
--spat_diff_loss_type minus_log, --head_type published, --attribution spatial_difference,
--nan_policy zero, --eval_test_every_epoch, --keep_all_best_checkpoints,
--no_padding_mask.

Tests

python -m pytest tests -q      # 58 tests

tests/fixtures/synthetic.py generates an ABIDE-shaped cohort, so the suite — including an
end-to-end main.py run — needs no cohort data, no GPU and no W&B account. Also run on an
NVIDIA GB10 in the NGC PyTorch container: 58/58, plus short step-2 runs with mixed precision on
and off and a step-3 masked-pretraining run. That GPU pass caught a defect in this patch (a
guarded NVTX helper that called itself — invisible on CPU, RecursionError with CUDA present),
now pinned by three tests.

Two claims withdrawn after checking

Recorded so a reader can tell what was verified from what was merely plausible: an apparent
ABIDE split KeyError (the column is renamed before the split call, so the released no-op is
correct) and an apparent _SITE_TR substring collision (no key is a substring of another, and
substring matching is required for ABIDE's suffixed site ids). Both reverted to released
behaviour; see the "Corrections made during this audit" section of AUDIT.md.