Skip to content

DICOMSeriesSelectorOperator Improvements #542

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

Merged

Conversation

bluna301
Copy link
Contributor

Following some requests from clinicians at CCHMC with regards to series selection, the following updates have been made to the DICOMSeriesSelectorOperator:

  • Matching of image orientation using the ImageOrientationPatient tag; the row and column cosines are used to compute the image orientation, which can be matched as Axial, Coronal, or Sagittal; logic is contained in the _match_image_orientation helper method
  • Relational operator (>, >=, <, <=, !=) matching for numerical DICOM tags; previously, the only relational operator supported was for exact matching (==)

Additionally, the following cleanup was performed:

  • Consolidation of all numerical DICOM tag matching logic to the _match_numeric_condition helper method
  • Updated some logging to be more descriptive and diagnostic; also updated log spacing to more easily visualize the matching process for each series
  • Grammar and spelling fixes

Signed-off-by: bluna301 <luna.bryanr@gmail.com>
@bluna301 bluna301 requested review from MMelQin, vikashg and dbericat May 14, 2025 01:19
Signed-off-by: bluna301 <luna.bryanr@gmail.com>
@bluna301
Copy link
Contributor Author

As discussed during MONAI Deploy App SDK meetings, it is possible that the value of the PatientPosition DICOM tag could influence the accuracy of the image orientation calculation contained in this PR. Previously, only the following PatientPosition values had been tested: HFP, HFS, FFP, and FFS.

Production CT and MRI cases at CCHMC were investigated from the past year; very few cases (<1%) had alternate orientations. These orientations (HFDL, HFDR, FFDL, and FFDR) were tested, and it was determined no update to the operator code is required to correctly determine the image orientation. As discussed during the MONAI meetings, it is likely uncommon for human imaging to have non Head First or Feet First orientations, given the size of patients.

This PR was updated to explicitly define the supported PatientPosition values (HFP, HFS, HFDL, HFDR, FFP, FFS, FFDL, and FFDR), and also throw NotImplemented errors if the PatientPosition tag is absent (this tag is Type 3 - Optional for CT and MR modalities), or has an orientation that has not been tested. The latter implementation was requested by @MMelQin during the 11JUN25 MONAI Deploy App SDK Meeting.

One note - when testing CCHMC scans, there were a few series whose orientation was misidentified due to the presence of a Scout/Localizer image at the beginning of the series that was in a different orientation compared to the rest of the series. This is a limitation of this operator's code - because it is using a single SOP instance of the DICOM Series to extract the ImageOrientationPatient tag value, Surveys, Scouts, Localizers, and mixed/interspersed DICOM Series are susceptible to this behavior.

Signed-off-by: bluna301 <luna.bryanr@gmail.com>
Copy link

@bluna301
Copy link
Contributor Author

Exclusion matching added for set types - ! flag indicates the exclusion match.

For example, the following rules will select series that contain "PRIMARY" and not "SECONDARY" in the ImageType tag:

{
    "name": "CT Series 5",
    "conditions": {
        "StudyDescription": "(.*?)",
        "Modality": "(?i)CT",
        "ImageType": ["PRIMARY", "!SECONDARY"]
    }
}

@MMelQin
Copy link
Collaborator

MMelQin commented Jun 24, 2025

I checked out bluna301:bl/dicom_series_selector_op_improvements to a local branch ss_op, and used Cursor to perform a review, in addition to manual review:

Review of Changes in Branch ss_op, local name

Based on the commit history and code analysis, this branch contains three main improvements to the DICOMSeriesSelectorOperator:

1. Exclusion Matching for Set Type

Key Enhancement: Added support for exclusion matching using the ! prefix in array/list conditions.

Changes Made:

# NEW: Enhanced list matching logic with inclusion/exclusion support
if isinstance(value_to_match, list):
    value_set = {str(element).lower() for element in value_to_match}
    # split inclusion and exclusion matches using ! indicator
    include_terms = {v for v in value_set if not v.startswith("!")}
    exclude_terms = {v[1:] for v in value_set if v.startswith("!")}
    matched = all(term in meta_data_list for term in include_terms) and all(
        term not in meta_data_list for term in exclude_terms
    )
elif isinstance(value_to_match, (str, numbers.Number)):
    v = str(value_to_match).lower()
    # ! indicates exclusion match
    if v.startswith("!"):
        matched = v[1:] not in meta_data_list
    else:
        matched = v in meta_data_list

Impact:

  • Excellent Feature: Allows users to specify both what must be included AND what must be excluded
  • Example Usage: "ImageType": ["PRIMARY", "!SECONDARY"] matches series with PRIMARY but excludes SECONDARY
  • Backward Compatible: Existing rules without ! continue to work

2. PatientPosition Tag Check for Image Orientation

Key Enhancement: Added robust PatientPosition validation for image orientation calculations.

Changes Made:

# NEW: PatientPosition validation before image orientation calculation
elif key == "ImageOrientationPatient":
    patient_position = series_attr.get("PatientPosition")
    if patient_position is None:
        raise NotImplementedError(
            "PatientPosition tag absent; value required for image orientation calculation"
        )
    if patient_position not in ("HFP", "HFS", "HFDL", "HFDR", "FFP", "FFS", "FFDL", "FFDR"):
        raise NotImplementedError(f"No support for PatientPosition value {patient_position}")
    matched = self._match_image_orientation(value_to_match, attr_value)

Impact:

  • Safety Improvement: Prevents crashes when PatientPosition is missing
  • Clear Error Messages: Explicit feedback about unsupported positions
  • DICOM Compliance: Validates against standard PatientPosition values

3. Series Selector Improvements (Multiple Enhancements)

3a. Advanced Numeric Matching

New Method: _match_numeric_condition() with comprehensive numeric operations:

def _match_numeric_condition(self, value_to_match, attr_value):
    # Supports:
    # - [val, ">"]: match if attr_value > val
    # - [val, ">="]: match if attr_value >= val  
    # - [val, "<"]: match if attr_value < val
    # - [val, "<="]: match if attr_value <= val
    # - [val, "!="]: match if attr_value != val
    # - [min_val, max_val]: inclusive range check
    # - "regex": regular expression match
    # - number: exact match

Impact:

  • Major Enhancement: Supports relational operators (>, >=, <, <=, !=)
  • Range Matching: Inclusive range checks [min, max]
  • Regex Support: Pattern matching for numeric values
  • Example: "SliceThickness": [2, ">"] matches thickness > 2mm

3b. Image Orientation Matching

New Method: _match_image_orientation() with mathematical orientation calculation:

def _match_image_orientation(self, value_to_match, attr_value):
    # Computes actual image orientation from ImageOrientationPatient tag
    # Supports: "Axial", "Coronal", "Sagittal" matching
    # Uses vector cross products and DICOM geometric constraints

Impact:

  • Advanced Feature: Calculates true image orientation from DICOM geometry
  • Mathematical Rigor: Uses numpy for vector calculations with proper validation
  • DICOM Standard: Follows DICOM coordinate system conventions
  • Example: "ImageOrientationPatient": "Axial" matches axial slices

3c. Enhanced Documentation

Updated Class Docstring:

  • Added clear explanation of exclusion matching
  • Updated examples showing new capabilities
  • Better formatting and structure

Overall Assessment

Strengths of These Changes:

  1. 🎯 Targeted Improvements: Each change addresses a specific real-world need
  2. 🔒 Backward Compatibility: All existing rules continue to work
  3. 📚 Comprehensive Documentation: Well-documented with examples
  4. 🧪 Mathematical Rigor: Image orientation uses proper vector mathematics
  5. ⚡ Enhanced Flexibility: Much more powerful matching capabilities
  6. 🛡️ Better Error Handling: Clear error messages for edge cases

Code Quality:

  • Maintainability: ✅ Good separation of concerns with helper methods
  • Readability: ✅ Clear method names and comprehensive docstrings
  • Robustness: ✅ Proper validation and error handling
  • Performance: ✅ Efficient implementation without unnecessary overhead

Potential Concerns:

  1. Complexity: The operator is becoming quite feature-rich - consider if some functionality should be split
  2. Testing: These advanced features would benefit from comprehensive unit tests
  3. Edge Cases: Image orientation calculation assumes well-formed DICOM data

Recommendations:

  1. ✅ Approve: These are solid, valuable improvements
  2. 📝 Add Tests: Create unit tests for the new matching logic
  3. 📖 Update Examples: Add more examples showing the new capabilities
  4. 🔍 Performance Testing: Test with large datasets to ensure good performance

Final Verdict: 9/10

This is an excellent set of improvements that significantly enhances the DICOM series selection capabilities while maintaining backward compatibility and code quality. The changes are well-implemented, properly documented, and address real clinical workflow needs.

@MMelQin
Copy link
Collaborator

MMelQin commented Jun 24, 2025

Cursor also suggested the following improvements to address the new magic strings, but I think

  • PatientPosition values are DICOM standards defined string literals, and in this code they are used in the private function, so fine as string literals in code
  • The image axial axis to anatomical axis names could be class constants, but they are also used as strings in selection rules in JSON, so class constant only solves half of the problem since they are not used in JSON text.
  • For tolerance, not usure if there is a need to be dynamic.

3. Magic Numbers & Constants

# Current:
tolerance = 1e-4

# Better:
class Constants:
    GEOMETRIC_TOLERANCE = 1e-4
    SUPPORTED_PATIENT_POSITIONS = ("HFP", "HFS", "HFDL", "HFDR", "FFP", "FFS", "FFDL", "FFDR")
    ALLOWED_ORIENTATIONS = {"Axial", "Coronal", "Sagittal"}

@MMelQin MMelQin merged commit 2ce415f into Project-MONAI:main Jun 27, 2025
5 checks passed
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

Successfully merging this pull request may close these issues.

2 participants