Reuse saved ICA component maps when generating component figures - #1356
Reuse saved ICA component maps when generating component figures#1356tsalo wants to merge 13 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1356 +/- ##
==========================================
+ Coverage 90.43% 90.52% +0.09%
==========================================
Files 31 31
Lines 5100 5107 +7
Branches 937 937
==========================================
+ Hits 4612 4623 +11
+ Misses 326 322 -4
Partials 162 162 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates tedana’s static component figure generation to reuse already-saved ICA component beta maps (instead of recomputing them) and adds joblib-based parallelization for per-component figure creation.
Changes:
- Update
comp_figuresto load"ICA components img"from the output registry rather than recomputing beta maps. - Refactor per-component plotting into a helper and run it in parallel via
joblib.Parallel. - Update workflow call sites to match the new
comp_figuressignature (and passn_threadsfromtedana_workflow).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
tedana/workflows/tedana.py |
Updates comp_figures call signature and passes n_threads. |
tedana/workflows/ica_reclassify.py |
Updates comp_figures call signature after removing ts/mask parameters. |
tedana/reporting/static_figures.py |
Implements loading cached component beta maps and parallel per-component figure generation. |
Comments suppressed due to low confidence (2)
tedana/workflows/ica_reclassify.py:596
comp_figuresnow reads spatial maps from "ICA components img" written byio.writeresultsusingmixing(after any TEDORT orthogonalization), but this call passesmixing_orig. That can make the displayed time series inconsistent with the displayed spatial maps whentedortis used.
reporting.static_figures.comp_figures(
component_table=component_table,
mixing=mixing_orig,
io_generator=io_generator,
png_cmap=png_cmap,
tedana/reporting/static_figures.py:402
comp_figuresbehavior changed significantly (now depends on an on-disk "ICA components img" and adds parallel execution), but there’s no test coverage for this function whiletedana.reportingdoes have tests. Consider adding a unit test that mocksnb.load/plot_componentto verify it uses the expected file key and iterates over all components (and optionally exercisesn_threads>1).
def comp_figures(component_table, mixing, io_generator, png_cmap, n_threads=1):
"""Create static figures that highlight certain aspects of tedana processing.
This includes a figure for each component showing the component time course,
the spatial weight map and a fast Fourier transform of the time course.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…edana into simplify-component-plots
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closes none.
Changes proposed in this pull request: