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

8134 Add unit test for responsive inference #8146

Merged
merged 23 commits into from
Nov 24, 2024

Conversation

Nic-Ma
Copy link
Contributor

@Nic-Ma Nic-Ma commented Oct 12, 2024

Fixes #8134 .

Description

This PR added unit test to cover the realtime inference with bundles.
And updated BundleWorkflow to support cyclically calling the run function with all components instantiated.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@ericspod
Copy link
Member

Hi @Nic-Ma, we haven't had time to discuss the ideas around realtime/streaming inference yet due to MICCAI, but I feel we should look into doing this with specialised versions of datasets and transforms to represent streams of incoming and outgoing items. This would implement streaming without modifying existing workflows if we can get it performant.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Oct 13, 2024

Hi @ericspod ,

Yes, I totally agree with you, I was thinking both the streaming way and repeatedly calling way.
Actually, for these 2 calling methods:
(1) pending IterableDataset in a separate thread for the real-time inference input streaming data.
(2) Repeatedly call BundleWorkflow.run() with replaced input data, just what I made the simple test case in this PR.

This PR is just a draft implementation for discussion, could you please help also write draft code for your idea and let's discuss later?
When you design the logic, please consider below typical use cases:
(1) Triton backend: https://github.com/triton-inference-server/python_backend?tab=readme-ov-file#usage
(2) MONAI deploy: https://github.com/Project-MONAI/monai-deploy-app-sdk/blob/main/monai/deploy/operators/monai_bundle_inference_operator.py#L277
(3) Holoscan inference: https://github.com/nvidia-holoscan/holohub/tree/main/tutorials/dicom_to_usd_with_monai_and_holoscan#monai-deploy--holoscan-pipeline

Thanks in advance.

monai/bundle/workflows.py Outdated Show resolved Hide resolved
@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 21, 2024

/build

Signed-off-by: YunLiu <[email protected]>
monai/bundle/workflows.py Outdated Show resolved Hide resolved
@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 22, 2024

/build

@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 22, 2024

/build

@KumoLiu KumoLiu requested a review from ericspod November 22, 2024 16:16
@KumoLiu KumoLiu changed the title [WIP] 8134 Add unit test for realtime inference 8134 Add unit test for responsive inference Nov 22, 2024
@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 22, 2024

Hi @ericspod, if you don't have any further comments, I will try to merge this one.

monai/bundle/workflows.py Outdated Show resolved Hide resolved
@ericspod
Copy link
Member

Hi @ericspod, if you don't have any further comments, I will try to merge this one.

Hi @KumoLiu I had just a minor change to add a docstring explaining what the method does and a bit on why it does it. Otherwise I'm good here.

Co-authored-by: Eric Kerfoot <[email protected]>
Signed-off-by: YunLiu <[email protected]>
@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 22, 2024

/build

@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 22, 2024

/build

1 similar comment
@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 22, 2024

/build

@KumoLiu KumoLiu merged commit d94df3f into Project-MONAI:dev Nov 24, 2024
28 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.

Add responsive inference support in bundle
4 participants