Skip to content

Pin the vector ISA in quicktest, and log what each host actually computes - #886

Merged
m-reuter merged 3 commits into
Deep-MI:devfrom
m-reuter:quicktest
Sep 15, 2026
Merged

m-reuter merged 3 commits into
Deep-MI:devfrom
m-reuter:quicktest

Conversation

@m-reuter

@m-reuter m-reuter commented Sep 15, 2026

Copy link
Copy Markdown
Member

Two quicktest runs of the same pipeline could not be compared, and nothing in the artifacts said
why. Sampling the GitHub runner pool explains it: the hardware varies enough to change the
arithmetic, and the CPU name does not predict how.

What the runner pool looks like

Eight ubuntu-latest runners, torch 2.7.1+cpu, fingerprinting a convolution, a matmul and a
softmax over bit identical inputs:

CPU seen ISA reported
AMD EPYC 9V74 3 AVX512 twice, AVX2 once
AMD EPYC 7763 2 AVX2
AMD EPYC 9V45 2 AVX512
Intel Xeon Platinum 8573C 1 AVX512

Two clean and separate dependencies:

  • convolution and softmax depend on the ISA alone. AMD and Intel at AVX512 produce identical
    results.
  • matmul depends on the vendor alone. Not relevant here: there is no torch.matmul,
    nn.Linear, einsum or bmm anywhere in the four model directories.

Counting only the two operations the networks use, two runs land in the same numerical class 53%
of the time
, so nearly half of all quicktest pairs are not comparable. Pinning the ISA collapses
them to one class.

Note the first row: the same CPU model appears with and without AVX512, because the hypervisor
masks it on some VMs. The model name therefore cannot decide whether two runs are comparable,
which is what motivates the second half of this PR.

Pinning, in CI only

Both docker run invocations in run-fastsurfer now set:

--env ATEN_CPU_CAPABILITY=avx2 --env ONEDNN_MAX_CPU_ISA=AVX2

Both are needed, because they cover disjoint operations: the first moves softmax and leaves
convolution alone, the second the reverse. avx2 rather than avx512 because the cap only works
downwards, so pinning high would be silently ignored on exactly the runners that cause the split.

Deliberately on the docker run and not in the Dockerfile. The penalty is roughly 50% on
convolution where AVX512 was available, so users keep the faster kernels and only CI pays. The
comment at the call site records that, so it is not moved into the image without knowing the cost.

A numerical fingerprint in the log

FastSurferCNN/host_info.py gains numerical_fingerprint(), which hashes a convolution and a
softmax over inputs built from exact integer arithmetic, so the inputs are identical on every host
and only the selected kernels can move the result:

Platform: Linux 6.17.0-1022-azure x86_64
CPU: AMD EPYC 9V74 80-Core Processor
CPU cores: 4
Torch 2.7.1+cpu, CPU capability AVX512
Numerical fingerprint: conv=a6cb179c42c9 soft=4a709ba8b6df
Thread limits: OMP_NUM_THREADS=4, ITK_GLOBAL_DEFAULT_NUMBER_OF_THREADS=4

This is the part that pays off repeatedly: comparing two runs now starts with a one line diff
instead of an investigation, and it identifies the numerical class exactly rather than by proxy.

The digest goes through int32 and struct rather than numpy, which costs about 4ms more and
means a missing numpy can never be what breaks a log line.

Where it is emitted

Once per log file, at the top, because it describes the host and the host does not change during a
run:

caller log
run_fastsurfer.sh deep-seg.log, in the header
recon-surf.sh recon-surf.log
recon-surfreg.sh its own log
long_prepare_template.sh its own log

The networks keep their existing torch line and do not repeat the fingerprint. Their line
carries the intra-op and inter-op thread counts, which do differ per process; that is why the
thread counts are per network while the fingerprint is not.

The header omits the thread counts for the same reason in reverse: there they would be torch's
defaults rather than the run's, which is why it did not report torch at all before.

This split relies on the fingerprint not depending on the thread count, since the header runs
before --threads is applied. Verified identical at 1, 2, 4 and 8 threads, and asserted by
test_does_not_depend_on_the_thread_count so a future torch version cannot quietly break it.

What this does not fix

Pinning closes the AVX512 against AVX2 split. It does not close everything: one observed pair
already agreed on convolution and softmax and still differed by a single voxel in the
segmentation, which then cascaded into different vertex counts and every surface derived number
after them. That residual is unexplained. Removing the large known variable is what will make it
visible and attributable next time, rather than confounded with the ISA.

Testing

Six new tests in test/utils/test_host_info.py: the fingerprint is stable within a process, has
the documented shape, stays on one line, does not depend on the thread count, reports plainly when
torch is absent, and, the one that matters most, changes when the ISA is capped. That last one
skips on arm64, where there are no wider kernels to cap, so it will first do real work on the
linux CI run.

test/utils + test/lint + test/shell   151 passed, 3 skipped
ruff                                  clean
bash -n                               clean on all shell scripts

Cost of the header now importing torch: 0.04s to 0.66s locally, once per run, against a pipeline
measured in tens of minutes. In exchange three redundant log lines disappear.

Notes for reviewers

The pinning will not be exercised by this PR's own CI. quicktest.yaml:236 resolves
run-fastsurfer@dev, so the pull request run uses the action from dev and the two environment
variables are absent. The fingerprint changes are repository source, baked into the image built
from this branch, so those are exercised. On the first dev run after merge, confirm the logs show
CPU capability AVX2 and that both resolutions report the same fingerprint.

Quicktest numbers will shift after this lands, because the pinned kernels are not the ones the
current reference was produced with. The reference is already stale and the suite already fails
continuously, so this makes nothing worse, but it is one more reason the refresh has to come after
this rather than before.

@m-reuter
m-reuter merged commit 11bcb60 into Deep-MI:dev Sep 15, 2026
6 checks passed
@m-reuter
m-reuter deleted the quicktest branch September 15, 2026 16:25
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.

1 participant