[FIX] staging ibma findings - #61
Merged
Merged
Conversation
Sixteen studysets cut from staging Neurostore were run through thirty specifications. Every fault below is one that run found; docs/staging-ibma-matrix.md records the evidence and the case count. Silently wrong results: - A p map has no sign, and NiMARE's only route from one returns an unsigned z, so an analysis carrying only a p map contributed an all-positive map. Measured on real staging data: 0.0% negative voxels against 37-59% for genuine z maps. `P map`, `P map (given null hypothesis)` and `1-P map` are no longer mapped to a NiMARE type. - A downloaded map with no finite non-zero voxel is dropped. Under an aggressive mask one empties the intersection and every output map returns NaN; under the liberal default it contributes to nothing while still counting as an analysis that did. 2 of 242 maps in this corpus were empty. Total failures: - A relative `result_dir` left relative image references, which NiMARE resolves against the same directory, so every map went missing. - Coordinate-based runs never had `apply_sample_sizes` applied, so a single analysis without a sample size failed the whole run. ALE sizes its kernel from it. Reachable in production for any NeuroVault-sourced studyset. - A null corrector argument, which is how the frontend spells "unset", reached the estimator: PermutedOLS rejects `voxel_thresh` outright and ALE thresholds against None. Between them that is the only FWE path either has. Unactionable errors: - A cancelling dependence group is named by the study and its analyses, with the correlation that caused it, instead of by NiMARE's internal group code. - A failure after the transform is reported from the estimator rather than from the submission, which previously described every converted analysis as one NiMARE could not convert. - An all-NaN result names how many analyses reached the estimator. - A filter naming a column the annotation lacks, and a selection matching nothing, each say so rather than surfacing as a KeyError or as a missing image type. - An image-based group comparison is refused before the maps are staged and before a reference studyset is downloaded, rather than after. `apply_filter` raising on an empty selection changes a documented behaviour; test_a_filter_column_no_note_carries_selects_nothing pins the new contract.
docs/staging-ibma-matrix.md records what the sixteen-studyset run found: the faults, the evidence for each, and what the staging image corpus can support at all. Three of its findings are Neurostore's rather than this repo's, and are recorded as such -- most importantly that of 134 staged z and t maps, 14 cover under 5% of the volume and 16 carry no negative value, and nothing in the API distinguishes a thresholded or one-sided map from an unthresholded one. docs/nimare-asks.md collects the seven changes wanted in NiMARE, each with a reproduction. Five came out of this run; two move here from docs/ibma.md, which now points at it rather than keeping a second list.
scripts/staging_matrix builds studysets and specifications from staging Neurostore searches and drives Runner over the combinations, one subprocess per cell, resumable, recording status, traceback, coverage and per-map statistics. Compose cannot create an image-based meta-analysis yet, so assembling the bundle is the only way to exercise the path. scripts/nimare_asks_repro.py reproduces the NiMARE asks against NiMARE alone -- synthetic NIfTIs and the public API, no compose-runner -- so each can be pasted into an issue as it stands. Both write their working directories in place; .gitignore covers them.
Five of the fixes in 5b3250b duplicated work that docs/nimare-asks.md 1-5 do one layer down. Assuming those land, removed here: - The cancelling-dependence-group message. Ask 4 drops the group and fits the rest rather than raising, so nothing reaches the translation. - The null-corrector-argument drop. Ask 5 drops them in `Corrector.__init__`, for every corrector rather than only the ones compose builds. - The empty-map half of the staged-image check. Ask 2 drops an image with no finite non-zero voxel and names it. - The "needs at least two analyses" sentence on an all-NaN result. Ask 3 raises before a fit can reach it. The analysis counts stay, since that message still covers a mask that leaves no overlap. Kept, because nothing upstream covers them: an unsigned p map is dropped rather than converted, since ask 1 warns and converts and a warning cannot be relied on by a service that uploads its results; and a file that is not a NIfTI is dropped while staging, since it otherwise reaches nibabel mid-fit unattributed. This trades a slightly worse coverage report for one place stating each fact -- an empty map is now excluded as "could not produce z", not as "has no finite non-zero voxel". It also means the runner requires a NiMARE newer than the pinned 0.21.0rc3; the pin comment says so.
The write-up described five workarounds this repository no longer carries. What was wrong and how it was diagnosed is unchanged; what was done about it is now NiMARE's for findings 2, 3 and 11, and partly for 5.
NiMARE ask 1 warns at the conversion and still converts, and where the analysis
carries a t map it recovers the sign from it. That is enough: two places
deciding a p map is unusable is one too many, and dropping costs a real study
where warning costs nothing.
Restores `p map`, `p map (given null hypothesis)` and `p` to
MAP_TYPE_TO_IMAGE_TYPE, and removes UNSIGNED_MAP_TYPES and
unusable_type_reason with them. `1-P map ("inverted" probability)` was never
mapped and still is not.
images.py is now two changes against main: staged paths are absolute, and a file
that will not parse as a NIfTI is dropped while staging rather than reaching
nibabel mid-fit.
rc4 carries docs/nimare-asks.md 1-8. Verified against it: each reproduction in scripts/nimare_asks_repro.py now shows the fixed behaviour, and the two staging studysets that could not be meta-analysed at all now complete -- `face` fits 16 of 18 analyses where a mirrored contrast pair used to end the run, and `motor` 9 of 15, with the empty upload dropped by name and the cancelling group excluded by study. Two test consequences, both from floors rc4 added rather than from anything wrong with it: - The coverage fixtures each fit a single surviving analysis, which rc4 refuses; and `_fit_and_describe` runs the workflow with the jackknife, which needs three. Each fixture gains usable studies until three survive. What every test is about -- that the report explains an exclusion -- is unchanged. - `test_an_all_nan_result_is_rejected` used non-overlapping maps under an aggressive mask, which rc4 now fails inside PyMARE before a result exists. The guard is still reachable by a studyset whose analyses all come from one study: it clears the floor on the analysis count but holds one dependence group, so every bag is skipped and every map is NaN. Retargeted there.
…-runner into fix/staging-ibma-findings
Removed comments regarding IBMA support and versioning.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I ran a few combinations of ibma workflows to see what broke.