Skip to content
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

Nested pipelines #54

Open
louisblankemeier opened this issue Apr 2, 2023 · 9 comments
Open

Nested pipelines #54

louisblankemeier opened this issue Apr 2, 2023 · 9 comments

Comments

@louisblankemeier
Copy link
Collaborator

Use nested pipelines to reduce code duplication - pipeline as an InferenceClass object

@louisblankemeier
Copy link
Collaborator Author

Initial version merged, have not tested multi-level nesting.

@fohofmann
Copy link
Contributor

regarding this, reusing 'converted_dcm.nii.gz' would be great. currently, the transformation to *.nii is done in nnUNet_predict_image. as the input directory or file_in in the pipeline itself is not changed, each prediction converts the dicoms to nifti again.

when the transformation of dcm -> nii is done by the pipeline itself, we maybe could also get rid of the adapted totalsegmentator?

@louisblankemeier
Copy link
Collaborator Author

Hi Felix,

Thanks for your feedback! I don't think that dicoms are being converted to niftis more than once? This only happens in the TotalSegmentator code, not nnUNet. If there's something you think I'm missing, could you send the files and lines of the first transformation and the second?

We are actually working on improving the nnUNet inference code and more of the processing will happen within C2C.

Thanks for the continued feedback, much appreciated!

@louisblankemeier
Copy link
Collaborator Author

louisblankemeier commented Apr 9, 2023

Maybe what you mean is that if TotalSegmentator inference code is called twice in the same pipeline, then conversion from DICOM to NIfTI would happen again? This is something that we are addressing, along with reusing segmentation predictions if a nested pipeline calls for a segmentation that has already been completed earlier in the pipeline.

@fohofmann
Copy link
Contributor

hi louis.
sorry for the misunderstanding.
if you would try to combine two segmentation jobs (e.g. spine+bodycomposition AND organs) in a single pipeline as far as I understand nnUNet_predict_image and L152 is called again each time.

Sorry again. Best, Felix

@louisblankemeier
Copy link
Collaborator Author

louisblankemeier commented Apr 9, 2023

Hi Felix,

Yes, I understand. That is absolutely true. Any computation that is completed in the first pipeline should be cached and reused by the second pipeline instead of redoing the computation. We are working on this currently!

Thank you!

Best,
Louis

@louisblankemeier
Copy link
Collaborator Author

louisblankemeier commented Apr 10, 2023

Hi Felix,

Just wanted to update you since you've been generous with your feedback. Yesterday, a collaborator ran into this exact issue while trying to combine two pipelines. So, I was forced to push a quick solution. Now, there is a module for converting from dcm to nifti within C2C. There are still other checks we will have to add to pipelines to ensure that we aren't redoing work that was completed by previous pipelines if pipelines are being nested.

Best regards,
Louis

@fohofmann
Copy link
Contributor

fohofmann commented Apr 11, 2023

haha, same idea, same class name, ... L6. sorry for the quick 'n dirty approach, its just to implement the pipeline in a more restricted HPC environment...

anyway, the missing return {} in SpineMuscleAdiposeTissueReport caused the pipeline to break afterwards, and thus, attaching an other inference object was not possible (see push).

@louisblankemeier
Copy link
Collaborator Author

haha that's awesome!!

Thanks so much for the PR; see the comments I left on it. Take care!

-Louis

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

No branches or pull requests

2 participants