StanfordMIMI / Comp2Comp

Computed tomography to body composition (Comp2Comp).
Apache License 2.0
61 stars 11 forks source link

Nested pipelines #54

Open louisblankemeier opened 1 year ago

louisblankemeier commented 1 year ago

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

louisblankemeier commented 1 year ago

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

fohofmann commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

haha that's awesome!!

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

-Louis