DOC: Add robustness analysis - #1129
Conversation
|
Thanks for opening this pull request! We have detected this is the first time you have contributed to NiMARE. Please check out our contributing guidelines. Of course, if you want to opt out this time there is no problem at all with adding your name later. You will be always welcome to add it in the future whenever you feel it should be listed. |
Reviewer's GuideThe example now demonstrates how to assess the stability of subtraction clusters under individual study dropouts by running a Jackknife diagnostic and inspecting its contribution table. Sequence diagram for subtraction Jackknife robustness analysissequenceDiagram
participant Example
participant Jackknife
participant MetaResult
participant DiagnosticResults
Example->>Jackknife: Jackknife(target_image, voxel_thresh, display_second_group)
Example->>Jackknife: transform(res_sub)
Jackknife->>MetaResult: evaluate study dropouts
MetaResult-->>Jackknife: subtraction cluster results
Jackknife-->>DiagnosticResults: return sub_diagnostic_results
Example->>DiagnosticResults: tables[diagnostic_table]
DiagnosticResults-->>Example: sub_jackknife_table
Example->>Example: head(10)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Hi , This PR resolves Issue #927 by integrating the missing Jackknife robustness analysis directly into the two-sample ALE subtraction example script. To achieve this, we added the We thoroughly verified the workflow by executing the script locally from top to bottom, confirming that all Monte Carlo permutations, diagnostic tables, and conjunction plots complete successfully without errors. You will notice we did not add a new external citation to the bibliography at the bottom; this is because Looking forward to your feedback! |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="examples/02_meta-analyses/08_plot_cbma_subtraction_conjunction.py" line_range="209-219" />
<code_context>
+ voxel_thresh=None,
+ display_second_group=True,
+)
+sub_diagnostic_results = jackknife.transform(res_sub)
+
+###############################################################################
+# Subtraction Jackknife Table
+# -----------------------------------------------------------------------------
+# We can inspect the contribution table to see how individual studies from
+# both groups influence the resulting subtraction clusters.
+sub_jackknife_table = sub_diagnostic_results.tables[
+ "z_desc-group1MinusGroup2Size_level-cluster_corr-FWE_method-montecarlo_diag-Jackknife_tab-counts_tail-positive"
+]
+sub_jackknife_table.head(10)
###############################################################################
# Conjunction analysis
</code_context>
<issue_to_address>
**issue:** The robustness diagnostic is run after `run_reports(res_sub, html_dir)`, so the generated HTML report does not contain the new subtraction Jackknife results. The example displays the table in the notebook-style output only after the report has already been written.
**Triggers:** When the example is executed to generate the documented HTML report.
**Suggested fix:** Move `run_reports(res_sub, html_dir)` after the Jackknife analysis, or explicitly regenerate the report after adding the diagnostic results.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: examples/02_meta-analyses/08_plot_cbma_subtraction_conjunction.py:219
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| sub_diagnostic_results = jackknife.transform(res_sub) | ||
|
|
||
| ############################################################################### | ||
| # Subtraction Jackknife Table | ||
| # ----------------------------------------------------------------------------- | ||
| # We can inspect the contribution table to see how individual studies from | ||
| # both groups influence the resulting subtraction clusters. | ||
| sub_jackknife_table = sub_diagnostic_results.tables[ | ||
| "z_desc-group1MinusGroup2Size_level-cluster_corr-FWE_method-montecarlo_diag-Jackknife_tab-counts_tail-positive" | ||
| ] | ||
| sub_jackknife_table.head(10) |
There was a problem hiding this comment.
issue: The robustness diagnostic is run after run_reports(res_sub, html_dir), so the generated HTML report does not contain the new subtraction Jackknife results. The example displays the table in the notebook-style output only after the report has already been written.
Triggers: When the example is executed to generate the documented HTML report.
Suggested fix: Move run_reports(res_sub, html_dir) after the Jackknife analysis, or explicitly regenerate the report after adding the diagnostic results.
jdkent
left a comment
There was a problem hiding this comment.
thanks @Achintyasingh412! please open/reference an existing issue before making a pull request. That allows us get alinged on what the issue is exactly before we try to fix it. That way we don't spend extra effort where it's not necessary. For example, I think this pull request could just be a comment on the relevant line that specifies the diagnostic, not a new section in this notebook.
| workflow = PairwiseCBMAWorkflow( | ||
| estimator=subtraction_estimator, | ||
| corrector=subtraction_corrector, | ||
| diagnostics=FocusCounter(voxel_thresh=0.01, display_second_group=True), |
There was a problem hiding this comment.
this is where the diagnostic is specified, we historically did not not run jackknife in the example because it took so long, I've since made some optimizations, so perhaps jackknife could be added here, but perhaps a note signalling jacnkknife could be used here would suffice.
|
Hi @jdkent, thank you for the guidance! I completely understand about linking the issue first—I'll make sure to do that for all future PRs. Regarding the diagnostic, thanks for pointing out that Pairwise CBMA Workflow handles it via the diagnostics parameter. Should I update the workflow to use Jackknife directly there now that it's optimized, or would you prefer a comment/note indicating where it can be swapped? |
|
responding to this comment in what I assume to be relevant to this issue: #1127 (comment) I would keep the diagnostic as focuscounter, there may be some cases that I don't cover with jackknife when the number of studies is low (like in this example), if you don't mind opening an issue, then someone (you if you want) can work on characterizing how that error arises and what a better guard/work around/error message would be. Thanks for looking into this! |
|
Done! Opened tracking issue #1130 to document the Jackknife distance matrix error during subtraction report generation. Thanks again for pointing it out |
This is a suggested pull request template for NiMARE.
It's designed to capture information we've found to be useful in reviewing pull requests.
If there is other information that would be helpful to include, please don't hesitate to add it!
Please also label your pull request with the relevant tags.
See here for more information and a list of available options:
https://github.com/neurostuff/NiMARE/blob/main/CONTRIBUTING.md#pull-requests
-->
Closes #927
Changes proposed in this pull request:
Summary by Sourcery
Document study-level robustness of subtraction meta-analysis results in the two-sample CBMA example.
Enhancements:
Documentation: