-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ENH] Skip visits for which processed images exist #1399
Merged
NicolasGensollen
merged 21 commits into
aramis-lab:dev
from
NicolasGensollen:implement-get_processed_images-for-pet-linear
Dec 4, 2024
Merged
[ENH] Skip visits for which processed images exist #1399
NicolasGensollen
merged 21 commits into
aramis-lab:dev
from
NicolasGensollen:implement-get_processed_images-for-pet-linear
Dec 4, 2024
Conversation
This file contains 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
AliceJoubert
approved these changes
Nov 29, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work @NicolasGensollen ! Just a few questions/remarks
clinica/pipelines/anatomical/freesurfer/longitudinal/correction/pipeline.py
Outdated
Show resolved
Hide resolved
…g on what the user is asking...
… matrix are found
…he pipeline some time ago
NicolasGensollen
force-pushed
the
implement-get_processed_images-for-pet-linear
branch
from
December 4, 2024 11:55
10bce0a
to
0492f90
Compare
NicolasGensollen
deleted the
implement-get_processed_images-for-pet-linear
branch
December 4, 2024 13:08
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #1079
This PR proposes to improve how pipelines determine which subject-session to skip.
Context
The base idea is to analyze the provided CAPS folder before running the processing pipeline and look for the expected output files. If those files are present for a given subject-session, then it won't be processed by the pipeline. This can save quite some time when re-running a pipeline.
This idea is definitely not new and some pipelines are already doing that (
T1Linear
and some Freesurfer longitudinal pipelines). Concretely, this is done in this method where the files are searched in the CAPS output folder:clinica/clinica/pipelines/t1_linear/anat_linear_pipeline.py
Lines 71 to 87 in 7436f44
The initial feature request of #1079 was to implement this logic for
PETLinear
too.The method above considers that, whatever the user is asking, if the cropped image is found for a subject-session, then it will be skipped. In other words, if the user is asking for the un-cropped image, or if interested in the transformation matrix rather than the image, then the pipeline will still skip the subject-session, which isn't great.
Instead, the method should look for both the transformation and the image corresponding to the user-provided parameters, and only skip the subject-session if both are present.
For
PETLinear
the idea is the same except that we need to take more parameters into account (tracer, SUVR...).Implementation
The diff is a bit long (sorry about that...), so here are the main ideas and implementation decisions:
There is a new dataclass called
Visit
, which is only a convenient way to represent a subject-session. I could also have used a NamedTuple but went for a frozen dataclass instead. Lists ofVisit
objects can be turned into sets (because they are frozen) and sorted (because they implement ordering): for example("sub-02", "ses-M006") > ("sub-01", "ses-M100")
or("sub-01", "ses-M006") > ("sub-01", "ses-M000")
. This class is inclinica.utils.bids
but might better fit somewhere else ?The previous method
get_processed_images
was renamedget_processed_visits
and returns a list ofVisit
for which processing is assumed to have been done.The skipping logic was moved from the concrete pipeline class to the base class (i.e. in the base
Pipeline
class inengine.py
). It is done in thedetermine_subject_and_session_to_process
method, which calls theget_processed_visits
abstract method to know what subjects and sessions to remove.get_processed_visits
has to be implemented in child classes as it is pipeline-specific.The PR proposes the implementation of this method for
T1Linear
andPETLinear
. Some "pattern builders" had to be implemented inclinica.utils.input_files
in order to be able to query new types of files (for example the transformation matrices in the pet-linear outputs, see functionpet_linear_transformation_matrix
).Most of the other changes are due to the implementation of the unit tests.
I had to modify a bit the CAPS generator to:
Because of the last point, the API of the CAPS generator had to change a bit (the desired pipelines to generate is not a list of strings anymore, but a dictionary mapping pipeline names to a dictionary of parameters). A lot of test files were only changed to use the new API.
Usage
Basically, through the CLI, when passing a CAPS folder with existing files, the user should get a warning displaying the visits that will be skipped: