-
Notifications
You must be signed in to change notification settings - Fork 48
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
DICOMSeriesSelectorOperator Enhancements #501
base: main
Are you sure you want to change the base?
DICOMSeriesSelectorOperator Enhancements #501
Conversation
Signed-off-by: bluna301 <[email protected]>
…ting parameter implemented Signed-off-by: bluna301 <[email protected]>
…p_folder declare location per pytype check Signed-off-by: bluna301 <[email protected]>
@vikashg @MMelQin - I appreciate your initial review of this PR, Elan provided me with your feedback earlier this week. Regarding your comment about sorting and using ImagePositionPatient instead of SOP instances, I apologize as perhaps it's not clear from my explanation what I am trying to accomplish with this functionality. I am not trying to sort the slices within the selected series - I am trying to sort the series themselves. For example, say My code is using the In my latest commit, I also made a change to the STL conversion operator to address a failed pytype check: Checking styles using pytype...
homebluna301monai-deploy-app-sdkmonaideployutilsimportutil.py20 DeprecationWarning pkg_resources is deprecated as an API. See httpssetuptools.pypa.ioenlatestpkg_resources.html
import pkg_resources
2024-09-06 163710 $ python3 -m pytype --version
2024.04.11
2024-09-06 163711 $ python3 -m pytype -j 16 --python-version=3.10
Computing dependencies
Analyzing 54 sources with 1 local dependencies
ninja Entering directory `.pytype'
[56] check monai.deploy.operators.stl_conversion_operator
FAILED homebluna301monai-deploy-app-sdk.pytypepyimonaideployoperatorsstl_conversion_operator.pyi
usrbinpython3 -m pytype.main --disable pyi-error,container-type-mismatch,attribute-error --imports_info homebluna301monai-deploy-app-sdk.pytypeimportsmonai.deploy.operators.stl_conversion_operator.imports --module-name monai.deploy.operators.stl_conversion_operator --platform linux -V 3.10 -o homebluna301monai-deploy-app-sdk.pytypepyimonaideployoperatorsstl_conversion_operator.pyi --analyze-annotated --nofail --quick homebluna301monai-deploy-app-sdkmonaideployoperatorsstl_conversion_operator.py
File homebluna301monai-deploy-app-sdkmonaideployoperatorsstl_conversion_operator.py, line 247, in convert Name 'temp_folder' is not defined [name-error]
For more details, see httpsgoogle.github.iopytypeerrors.html#name-error
ninja build stopped cannot make progress due to previous errors.
Leaving directory '.pytype'
pytype check failed! I moved the declaration of |
Thanks @bluna301. I am looking at the code. Give me one more day and I will be able to comment on your PR. |
Hey @bluna301, I understand the logic now and it makes sense too. Sometimes, I have also faced this issue and generally the series with highest number of dicoms is the right series. |
Signed-off-by: bluna301 <[email protected]>
Quality Gate passedIssues Measures |
@vikashg Thank you for your review! I agree with your suggestion, it would be good to make it more explicit to the user what is going on during when multiple series are matched. I added an info log for when sorting occurs, and also added some additional detail in the documentation of this operator to make it explicit that enabling this feature will "return" (i.e. sort and place first in the returned List) the series with the highest # of DICOM images. I have tested the new series selection functionality locally, as well as the minor change to the STL operator, with no issues. Please let me know if you or @MMelQin would like to test out the functionality before merging the PR. |
Addressing some points raised in Issue #204.
Enhanced Matching for Numerical DICOM Tags
In addition to the previously supported exact matching, the following is now supported for numerical (i.e. integer and float type DICOM VRs, including IS and DS VRs) tags:
I have also updated the example sample selection rules to include a 3rd selection ("CT Series 3"); this new selection includes an example of union matching for set type (functionality already present, but an example was not previously provided), as well as an example of the newly-added numerical range matching:
Sort by SOP Instance Count
In reviewing ~30 CT A/P scans from our hospital, filtering by StudyDescription, Modality, ImageType, and SliceThickness narrows down to a single, desired series in about 85% of cases (CT, Axial, 3-5 mm ST). For the remaining 15%, it is often the case that, of the series that pass the initial filtering, the series that is desired for inference is the one with the most number of DICOM images present (i.e. highest number of SOP instances). This is often due to the filtered series with more DICOM images having a higher likelihood of having the complete A/P anatomy present, as well as the presence of short series (e.g. Smart Prep Series) which technically pass the filters but are not desired for inference.
With this observation, I have implemented a new default parameter,
sort_by_sop_instance_count
, which, whenTrue
, and when multiple series pass the selection criteria (all_matched=True
), will sort the selected series in descending SOP instance count, i.e. will list the filtered DICOM series with the most DICOM images first. This will give a higher probability that, despite downstream operators (the MONAIBundleInferenceOperator, I believe) being limited to only performing inference on the first selected series even if multiple series are selected, inference is performed on the desired series.sort_by_sop_instance_count
is implemented in a very similar way toall_matched
- as a default parameter with a default value ofFalse
.