MONAISegInferenceOperator Additional Arguments

[bluna301]

When translating the MONAI CT TotalSegmentator MONAI Bundle to a MAP, it was determined that the segmentations produced by the MONAI Bundle and MAP were not exactly aligned (i.e. DICE != 1 for all organs).

This seems to be due to the MONAISegInferenceOperator not accepting all the possible arguments that are accepted by the sliding_window_inference method, specifically mode and padding_mode for this Bundle. Once a custom operator that accepted these input arguments was implemented into the MAP pipeline, the DICE for all organs was ~=0.99 when comparing to the Bundle.

This initial commit includes the logic for accepting and implementing mode and padding_mode arguments. Happy to discuss further if the additional missing sliding_window_inference arguments should be added as well.

[MMelQin]

It is good to add these couple parameters explicitly on the __init__ function of this operator. In addition, we can also consider supporting all other params on the MONAI sliding_window_inference function in a implicitly, i.e. by passing the (filtered) kwargs down to the function. The filtering is to ensure no keys in the kwargs are duplicates of the explicitly supported params.

A test example

>>> def test(a: str = "it", b: str = "is Good", c: bool = True, d: dict = None, *args: Any, **kwargs: Any):
...     print(f"{a} {b}")
...     bool_val = "True" if c else "False"
...     print(f"The boolean val is {bool_val}")
...     if d:
...         for key, val in d.items():
...             print(f"{key}: {val}")
... 
>>> mine={'c': True, 'extra': 'unknown_to_fn'}
>>> test('a' = 'Test', 'b' = 1, **mine)
>>> test(a = 'Test', b = 1, **mine)
Test 1
The boolean val is True
>>> mine={'b': 2, 'c': False, 'extra': 'unknown_to_fn'}
>>> test(a = 'Test', b = 2, **mine)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __main__.test() got multiple values for keyword argument 'b'
>>> mine={'b': 3, 'c': True, 'extra': 'unknown_to_fn'}
>>> test(a = 'Test', **mine)
Test 3
The boolean val is True
>>>

A function to filter out the params in the __init__ from the **kwargs can be implemented as

    @staticmethod
    def filter_sliding_window_params(**kwargs) -> Dict[str, Any]:
        """
        Returns a dictionary of named parameters of the sliding_window_inference function
        that are not in the __init__ of this class, or explicitly used on calling sliding_window_inference.
        """

        # Need to import inspect if not yet
        init_params = inspect.signature(MonaiSegInferenceOperator).parameters

        filtered_params = {}
        for name, val in kwargs.items():
            if name not in init_params and name not in ["inputs", "predictor", "args", "kwargs"]:
                filtered_params[name] = val
        return filtered_params

This function can be called in the init, and the resultant dictionary saved as an attribute, say self._implicit_params. When calling the sliding_window_inference, just need to add **self._implicit_params. The description of this class can also be updated to explain the use of the **kwargs.

[bluna301]

@MMelQin many thanks for your review! Completely agree with all of your suggestions, and appreciate the base filtering function you provided for handling the implicit parameters. I will work on integrating your suggestions into the operator.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[bluna301]

@MMelQin PR has been updated - please see comments for detailed updates.

I did have one remaining question about the device parameter - I see this is hardcoded into the operator, and this is also a sliding_window_inference parameter. Is this something we need to address?

[MMelQin]

LGTM. Thanks for expanding the support.

[bluna301]

Hi @MMelQin - sorry to bring this one back up. I was testing with passing in a device input as type torch.device() and received the following error:

infer_operator = MonaiSegInferenceOperator(
    self.fragment,
    roi_size=(96, 96, 96),
    pre_transforms=pre_transforms,
    post_transforms=post_transforms,
    overlap=0.25,
    app_context=self.app_context,
    model_name="",
    inferer=InfererType.SLIDING_WINDOW,
    sw_batch_size=1,
    mode=BlendModeType.GAUSSIAN,
    padding_mode=PytorchPadModeType.REPLICATE,
    device=torch.device("cuda" if torch.cuda.is_available() else "cpu"),
    model_path=self.model_path
)

[error] [gxf_wrapper.cpp:118] Exception occurred for operator: 'ct_totalseg_op' - RuntimeError: python object could not be converted to Arg

At:
  /home/bluna301/miniconda3/envs/ct-totalsegmentator/lib/python3.9/site-packages/holoscan/core/__init__.py(389): __init__
  /home/bluna301/ct-totalsegmentator-map/my_app/inference_operator.py(26): __init__
  /home/bluna301/ct-totalsegmentator-map/my_app/monai_seg_inference_operator.py(190): __init__
  /home/bluna301/ct-totalsegmentator-map/my_app/ct_totalseg_operator.py(202): compute

[error] [entity_executor.cpp:596] Failed to tick codelet ct_totalseg_op in entity: ct_totalseg_op code: GXF_FAILURE
[warning] [greedy_scheduler.cpp:243] Error while executing entity 28 named 'ct_totalseg_op': GXF_FAILURE
[info] [greedy_scheduler.cpp:401] Scheduler finished.
[error] [program.cpp:580] wait failed. Deactivating...
[error] [runtime.cpp:1649] Graph wait failed with error: GXF_FAILURE
[warning] [gxf_executor.cpp:2428] GXF call GxfGraphWait(context) in line 2428 of file /workspace/holoscan-sdk/src/core/executors/gxf/gxf_executor.cpp failed with 'GXF_FAILURE' (1)
[info] [gxf_executor.cpp:2438] Graph execution finished.
[error] [gxf_executor.cpp:2446] Graph execution error: GXF_FAILURE
Traceback (most recent call last):
  File "/home/bluna301/miniconda3/envs/ct-totalsegmentator/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/bluna301/miniconda3/envs/ct-totalsegmentator/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/home/bluna301/ct-totalsegmentator-map/my_app/__main__.py", line 25, in <module>
    CTTotalSegmentatorApp().run()
  File "/home/bluna301/ct-totalsegmentator-map/my_app/app.py", line 61, in run
    super().run(*args, **kwargs)
  File "/home/bluna301/ct-totalsegmentator-map/my_app/ct_totalseg_operator.py", line 202, in compute
    infer_operator = MonaiSegInferenceOperator(
  File "/home/bluna301/ct-totalsegmentator-map/my_app/monai_seg_inference_operator.py", line 190, in __init__
    super().__init__(fragment, *args, **kwargs)
  File "/home/bluna301/ct-totalsegmentator-map/my_app/inference_operator.py", line 26, in __init__
    super().__init__(fragment, *args, **kwargs)
  File "/home/bluna301/miniconda3/envs/ct-totalsegmentator/lib/python3.9/site-packages/holoscan/core/__init__.py", line 389, in __init__
    _Operator.__init__(self, self, fragment, *args, **kwargs)
RuntimeError: python object could not be converted to Arg

I believe this behavior is due to how Holoscan converts kwargs - I believe only JSON-y types will get successfully forwarded as GXR Args from Python --> C++ layer, and torch.device() is not convertible.

Because we already store the (filtered) kwargs as self._implicit_params, we could remove **kwargs from the super().__init__() call to resolve this error:

super().__init__(fragment, *args)

If you think this is acceptable, I can commit this fix. Please let me know if you have any concerns about this fix.

[MMelQin]

Hi @MMelQin - sorry to bring this one back up. I was testing with passing in a device input as type torch.device() and received the following error:

infer_operator = MonaiSegInferenceOperator(
    self.fragment,
    roi_size=(96, 96, 96),
    pre_transforms=pre_transforms,
    post_transforms=post_transforms,
    overlap=0.25,
    app_context=self.app_context,
    model_name="",
    inferer=InfererType.SLIDING_WINDOW,
    sw_batch_size=1,
    mode=BlendModeType.GAUSSIAN,
    padding_mode=PytorchPadModeType.REPLICATE,
    device=torch.device("cuda" if torch.cuda.is_available() else "cpu"),
    model_path=self.model_path
)

[error] [gxf_wrapper.cpp:118] Exception occurred for operator: 'ct_totalseg_op' - RuntimeError: python object could not be converted to Arg

At:
  /home/bluna301/miniconda3/envs/ct-totalsegmentator/lib/python3.9/site-packages/holoscan/core/__init__.py(389): __init__
  /home/bluna301/ct-totalsegmentator-map/my_app/inference_operator.py(26): __init__
  /home/bluna301/ct-totalsegmentator-map/my_app/monai_seg_inference_operator.py(190): __init__
  /home/bluna301/ct-totalsegmentator-map/my_app/ct_totalseg_operator.py(202): compute

[error] [entity_executor.cpp:596] Failed to tick codelet ct_totalseg_op in entity: ct_totalseg_op code: GXF_FAILURE
[warning] [greedy_scheduler.cpp:243] Error while executing entity 28 named 'ct_totalseg_op': GXF_FAILURE
[info] [greedy_scheduler.cpp:401] Scheduler finished.
[error] [program.cpp:580] wait failed. Deactivating...
[error] [runtime.cpp:1649] Graph wait failed with error: GXF_FAILURE
[warning] [gxf_executor.cpp:2428] GXF call GxfGraphWait(context) in line 2428 of file /workspace/holoscan-sdk/src/core/executors/gxf/gxf_executor.cpp failed with 'GXF_FAILURE' (1)
[info] [gxf_executor.cpp:2438] Graph execution finished.
[error] [gxf_executor.cpp:2446] Graph execution error: GXF_FAILURE
Traceback (most recent call last):
  File "/home/bluna301/miniconda3/envs/ct-totalsegmentator/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/bluna301/miniconda3/envs/ct-totalsegmentator/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/home/bluna301/ct-totalsegmentator-map/my_app/__main__.py", line 25, in <module>
    CTTotalSegmentatorApp().run()
  File "/home/bluna301/ct-totalsegmentator-map/my_app/app.py", line 61, in run
    super().run(*args, **kwargs)
  File "/home/bluna301/ct-totalsegmentator-map/my_app/ct_totalseg_operator.py", line 202, in compute
    infer_operator = MonaiSegInferenceOperator(
  File "/home/bluna301/ct-totalsegmentator-map/my_app/monai_seg_inference_operator.py", line 190, in __init__
    super().__init__(fragment, *args, **kwargs)
  File "/home/bluna301/ct-totalsegmentator-map/my_app/inference_operator.py", line 26, in __init__
    super().__init__(fragment, *args, **kwargs)
  File "/home/bluna301/miniconda3/envs/ct-totalsegmentator/lib/python3.9/site-packages/holoscan/core/__init__.py", line 389, in __init__
    _Operator.__init__(self, self, fragment, *args, **kwargs)
RuntimeError: python object could not be converted to Arg

I believe this behavior is due to how Holoscan converts kwargs - I believe only JSON-y types will get successfully forwarded as GXR Args from Python --> C++ layer, and torch.device() is not convertible.

Because we already store the (filtered) kwargs as self._implicit_params, we could remove **kwargs from the super().__init__() call to resolve this error:

super().__init__(fragment, *args)

If you think this is acceptable, I can commit this fix. Please let me know if you have any concerns about this fix.

Good catch @bluna301! This is actually a common issue with all the subclass of the base operator due to the failure in py_object_to_arg.

I initially also thought we could avoid using **kwargs on calling base operators __init__, but examining its code revealed that it also allows and explicitly looks for kwargs of type Condition and Resource to assign them to conditions and resources attributes, besides looking for them in the *args list. It also does the same for the name and description kwargs. So, it would not be safe to completely remove **kwargs on initializing the base.

I discussed this issue with other Holoscan SDK developers. As of now, there is no built-in attributes or a bool function to check if a type can be converted, and using try/catch with py_object_to_arg as a test is too expensive.

So, this leaves us with the approach to have yet another filer to distill the **kwargs with an allowed list. This list could be inferred from the Holoscan code (as it is not explicitly stated in the API doc). Generally, primary types, list/array, and the few known types are allowed. The following are the proposed changes including a update to the import list,

from monai.deploy.core import AppContext, Condition, ConditionType, Fragment, Image, OperatorSpec, Resource

and on calling super init

        # Remove arguments from the kwargs that cannot be converted by the base class __init__.
        # For now, filter out any object type that are not Condition or Resource or primitive types.
        # This will be revisited when there is a better way to handle this.
        allowed_types = (str, int, float, bool, bytes, List, Sequence, Condition, Resource)
        filtered_kwargs = {k: v for k, v in kwargs.items() if isinstance(v, allowed_types)}

        super().__init__(fragment, *args, **filtered_kwargs)

Also, I needed to add Resource in the monai.deploy.core's init.py to explicitly import resource to quiet mypy complaints.

from holoscan.core import Application, Condition, ConditionType, Fragment, Operator, OperatorSpec, Resource

Thanks.

[bluna301]

@MMelQin - I updated the PR following your guidance, but made some additional tweaks. Admittedly there are some signficant changes in this commit - I default to you regarding implementing all of these changes.

I checked all argument types explicitly defined by sliding_window_inference; those that would fail Holoscan conversion, I defined as explicit parameters in the operator (these were sw_device, device, and process_fn). Now, this operator will accept all possible explicitly defined sliding_window_inference args, which was the initial intention of this PR. Through my testing, I saw that these types failed conversion:

• None
• device (torch.device)
• Callable (function)
• 3D (and higher) NumPy arrays
• range()

I added the Holoscan conversion filter to the previously defined filter_sw_kwargs method. Thus, this method now filters out any repeat/internally defined parameters, as well as filters out any arguments whose type would fail Holoscan conversion. I did find that lists/tuples with mixed types and nested list/tuples would fail conversion, but I think some onus should be on the user to enter expected SWI inputs/formats (we state as much in the comments). I left out 1D and 2D numpy arrays, even those these are accepted by Holoscan.

I noticed that some of the setter type checks (e.g. overlap, sw_batch_size) were not occurring in the operator during runtime due to how explicit parameters were defined (public/private delineations were used liberally). I updated parameters to be private (_) or public based on their behavior, and now type checking at runtime is occurring.

I updated the existing overlap and inferer checks; the former was not aligned with the validation checks present in sliding_window_inference, while the latter was missing the string type check.

Lastly, torch was changed from an optional to a required import - the existing operator has several places where this dependency is needed hard-stop.

[MMelQin]

Thanks @bluna301. I'll be reviewing the changes. Also, I've created a feature enhancement on Holoscan SDK, to support a Python function to test if an arg can be converted.

[Copilot]

Pull Request Overview

This PR enhances the MONAISegInferenceOperator to accept additional arguments from MONAI's sliding_window_inference function, specifically mode and padding_mode, to improve alignment between MONAI Bundle and MAP segmentation outputs.

• Adds support for additional sliding window inference parameters via **kwargs
• Implements a filtering mechanism to validate and forward compatible arguments to sliding_window_inference
• Expands explicit parameter support for sw_device, device, and process_fn with proper type validation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
monai/deploy/operators/monai_seg_inference_operator.py	Main implementation adding parameter filtering, new explicit parameters, improved type validation, and updated inference calls
monai/deploy/core/init.py	Adds Resource import to support expanded parameter types

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[MMelQin]

The enhancements look good to me. Please see the proposed changes to split kwargs into inference use and for base operator init, which I believe is the simplest ways to separate concerns.

[bluna301]

@MMelQin thanks for your review - I will work on implementing the changes.

If I understand correctly, the filtered SWI implicit params (filtered_params) would be passed as a kwarg input during the sliding_window_inference call, while filtered_base_init_params would be passed onto the base __init__, leaving the code looking something like this:

# def __init__():
    self._filtered_params, self._filtered_base_init_params = self.filter_sw_kwargs(**kwargs) # Filter keyword args
    
    # Pass filtered base init params
    super().__init__(fragment, *args, **self._filtered_base_init_params)

# def compute_impl():

    d[self._pred_dataset_key] = sliding_window_inference(
                            **self._filtered_params,  # Additional sliding window arguments
                        )

If self._filtered_params will never be passed for Holoscan conversion, I would propose eliminating the three new explicit params (sw_device, device, and process_fn) I added in my last commit - these were originally made explicit to avoid Holoscan conversion, but if we won't be passing them at all, we can safely remove these from the operator. I would prefer this to not pollute the operator with additional explicit params, and would have the added bonus of not adding any explicit params in this PR at all, which would hopefully minimally change documentation and reduce user confusion.

Let me know if you disagree, but will plan on removing these three explicit params otherwise.

[MMelQin]

@MMelQin thanks for your review - I will work on implementing the changes.

If I understand correctly, the filtered SWI implicit params (filtered_params) would be passed as a kwarg input during the sliding_window_inference call, while filtered_base_init_params would be passed onto the base __init__, leaving the code looking something like this:

# def __init__():
    self._filtered_params, self._filtered_base_init_params = self.filter_sw_kwargs(**kwargs) # Filter keyword args
    
    # Pass filtered base init params
    super().__init__(fragment, *args, **self._filtered_base_init_params)

# def compute_impl():

    d[self._pred_dataset_key] = sliding_window_inference(
                            **self._filtered_params,  # Additional sliding window arguments
                        )

If self._filtered_params will never be passed for Holoscan conversion, I would propose eliminating the three new explicit params (sw_device, device, and process_fn) I added in my last commit - these were originally made explicit to avoid Holoscan conversion, but if we won't be passing them at all, we can safely remove these from the operator. I would prefer this to not pollute the operator with additional explicit params, and would have the added bonus of not adding any explicit params in this PR at all, which would hopefully minimally change documentation and reduce user confusion.

Let me know if you disagree, but will plan on removing these three explicit params otherwise.

Yes, I agree with your assessment.

[bluna301]

Great, thanks Ming!

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[MMelQin]

Thanks for the refinement to make it the best it can be.
I have also tested the changes in existing examples successfully. Ready to merge.