Skip to content

BUG: Keep tract indices when applying the length threshold - #122

Open
demianw wants to merge 1 commit into
masterfrom
fix-length-threshold-indices
Open

demianw wants to merge 1 commit into
masterfrom
fix-length-threshold-indices

Conversation

@demianw

@demianw demianw commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

What was wrong

With --length_threshold > 0, compute_tract_label_indices removed the short tracts from its local copy of the tract list before building the label maps. The keys of the four maps (crossing_tracts_labels, crossing_labels_tracts and both ending maps) were therefore positions in the filtered list, while TractographySpatialIndexing computes the tract bounding boxes and endpoint positions, and the CLI saves the tracts, from the full list. Query results mixed up unrelated tracts.

Reproduction on the test dataset (IIT3mean_left_hemisphere_small.trk, 6783 tracts, Desikan atlas), threshold at the 30th length percentile (52.5 mm), through the Python API on master:

  • cortico_spinal.left (wmql_1_cst.qry): 385 tracts, of which 111 are shorter than the threshold and only 25 are in the correct result.
  • wmql_2_uf.qry: KeyError: 4748 from anterior_of (relative terms select tracts from the full-length bounding boxes, then look them up in the filtered maps).
  • all_commissural: 0 (correct by chance: the 3 tracts are all shorter than the threshold).

The change

  • The label maps are keyed by the position of each tract in the full tractography: compute_label_crossings and compute_label_endings_start_end take the original indices of the tracts they index.
  • Tracts below the threshold are left out of all four maps, so neither label terms nor endpoints_in/both_endpoints_in can return them.
  • Their rows in tract_bounding_boxes and tract_endpoints_pos are set to NaN (rows stay aligned with the tractography). Comparisons with NaN are false, so relative terms (anterior_of, ...) do not select them either.

Testing

  • New tract_querier/tests/test_tract_label_indices.py: a synthetic atlas (three slabs) with 7 tracts, and queries covering label terms, endpoints_in, both_endpoints_in, only, not, relative terms and endpoints_in of a relative term. It asserts that for every query result(threshold) == {i in result(0) : length(i) >= threshold}. It fails on master (KeyError) and passes with this change.
  • Test dataset, all three query files, through the API, threshold at percentiles 0/30/60: the same identity holds for all 5 saved queries at 30 and 60. With the change: cortico_spinal.left 385 / 385 / 385 (none below the threshold), uncinate.left 102 / 102 / 52, all_commissural 3 / 0 / 0. Results at threshold 0 are unchanged.
  • python -m pytest -q -p no:cacheprovider tract_querier: 43 passed, 1 skipped (the new test included).

Merge order

Independent of the other open PRs. #129 (#50, single-point tracts) and #140 (#117, deduplicating the label/tract inversion) are stacked on this branch and must be merged after it. #135 (#100) is independent.

Closes #46

🤖 Generated with Claude Code

compute_tract_label_indices dropped the tracts shorter than
length_threshold from its local list before indexing, so the keys of the
four label maps were positions in the filtered list, while the tract
bounding boxes, the endpoint positions and the saved tracts use positions
in the full tractography. Queries combined unrelated tracts, returned
tracts below the threshold, and relative terms raised KeyError.

Key the label maps by the position in the full tractography and leave
the short tracts out of all of them. Set the bounding boxes and endpoint
positions of the left-out tracts to NaN so that relative terms do not
select them either.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

Use length threshold parameter consistently

1 participant