Skip to content

Return the Dice overlap, name the CI artifacts per run, and say what actually failed - #879

Merged
m-reuter merged 7 commits into
Deep-MI:devfrom
m-reuter:quicktest
Sep 11, 2026
Merged

Return the Dice overlap, name the CI artifacts per run, and say what actually failed#879
m-reuter merged 7 commits into
Deep-MI:devfrom
m-reuter:quicktest

Conversation

@m-reuter

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

Copy link
Copy Markdown
Member

Three things that made a quicktest run hard to read: the downloaded files could not be told apart,
a failure said only that something was out of range, and dice_score did not return Dice.

dice_score returns the Dice overlap

FastSurferCNN.utils.metrics.dice_score returned the Dice dissimilarity, best 0, while
DiceScore.compute in the same module returned the overlap, best 1. Its docstring claimed both, one
line apart. It now returns the overlap in both the scipy and the numpy branch.

Two call sites. CerebNet/utils/meters.py writes the result into a column labelled "Dice" and was
reporting 1 - Dice, so 0.02 appeared where 0.98 was meant; that is now correct. In the quicktest
the gate flipped from "close to 0" to "close to 1", which also makes is_low_dice mean what it
says. The yaml tolerances keep their exact meaning, a distance from a perfect overlap, so no data
file changed and no verdict changed. dice.csv from --collect_csv now holds the overlap, so a
file collected before this is not comparable to one collected after.

Failure messages

An assertion now names the worst offender and how far off it was, with the value and its limit in
the same units:

Dice scores in aseg.mgz are not within range! 7 of 41 labels below their minimum,
worst CC_Central at Dice 0.2263 (min 0.9900).
The stats tests gained the same, and they now separate a structure or measure that is missing from
the test subject from one whose values drifted, since the first is the larger failure:

The differences for some structures in aseg.stats exceed limits!
1 of 45 structures are missing from the test subject: Left-Lateral-Ventricle.
35 of 45 structures exceed their limit: Left-Inf-Lat-Vent, Left-Cerebellum-White-Matter, ...
A missing measure used to raise KeyError while the message was being built rather than report
anything. The reported deviation is now relative to the same value the threshold is applied to, so
it can be read against the limit. The CSV collection also gained the zero guard the table path
already had, so two measures that are both 0 no longer divide by zero.

Artifact names

Every artifact carries the short commit sha and the subject, and the JUnit file is named after its
artifact, so results from several runs can sit in one download folder:

fastsurfer-52e7dfa5-1000um.zip processed data
fastsurfer-52e7dfa5-junit-1000um.zip -> fastsurfer-52e7dfa5-junit-1000um.xml
fastsurfer-52e7dfa5-0800um.zip processed data
fastsurfer-52e7dfa5-junit-0800um.zip -> fastsurfer-52e7dfa5-junit-0800um.xml

The subject is written in micrometres, zero padded, because the name must contain no dot: GitHub
serves an artifact as <name>.zip, and Safari on macOS reads .8mm.zip as the extension, so a
0.8mm artifact arrived unpacked as 0. subject-id is unchanged, since it is also the subject
directory name and has to match the reference tarball. The new artifact-id input defaults to
subject-id, and run-tests passes it on to load-processed so the download asks for the name
that was uploaded. run-tests resolves the artifact name first and derives the JUnit path from it,
which replaces its junit-file input with junit-dir. unittest.yaml follows the same scheme and
drops from the full sha to eight characters.

Header comparison

test_image_headers compares the NIfTI qform as a transform rather than field by field. A rotation
has two quaternion representations, q and -q, and two headers writing the same geometry with
opposite signs were reported as different. Headers with qform_code 0 describe no transform and
take their geometry from the sform, so their leftover fields are skipped too. MGH headers have no
qform and stay exactly compared.

Note for review

The workflows resolve run-fastsurfer, run-tests and load-processed at @dev, so this PR's own
CI runs the previous versions of those actions and none of the naming changes are exercised here.
The artifact names need one look on the first dev run after merge.

@m-reuter m-reuter changed the title Name the CI artifacts per run, and say in the assertion what actually failed Return the Dice overlap, name the CI artifacts per run, and say what actually failed Sep 11, 2026
@m-reuter
m-reuter merged commit b688279 into Deep-MI:dev Sep 11, 2026
5 checks passed
@m-reuter
m-reuter deleted the quicktest branch September 11, 2026 20:50
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