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

Add recursive lookup for better ensemble disconnectivity reporting #239

Merged
merged 2 commits into from
Aug 8, 2023

Conversation

GuanLuo
Copy link
Contributor

@GuanLuo GuanLuo commented Aug 4, 2023

No description provided.

dyastremsky
dyastremsky previously approved these changes Aug 4, 2023
Copy link
Contributor

@dyastremsky dyastremsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice change! Very clean.

Approved your related tests here too: triton-inference-server/server#6149

Copy link
Contributor

@oandreeva-nv oandreeva-nv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! I am curious, if this is an issue, then would it make sense to provide a sanity check tool for ensembles ?

@GuanLuo GuanLuo force-pushed the gluo-ensemble_message branch from 1bf1fe8 to bc4d54e Compare August 8, 2023 00:28
@GuanLuo
Copy link
Contributor Author

GuanLuo commented Aug 8, 2023

I think we could, it is relatively easy to extract the static checking part of the ensemble config.

@GuanLuo GuanLuo merged commit 669a7cd into main Aug 8, 2023
@GuanLuo GuanLuo deleted the gluo-ensemble_message branch August 8, 2023 17:06
@dyastremsky
Copy link
Contributor

dyastremsky commented Aug 8, 2023

I think we could, it is relatively easy to extract the static checking part of the ensemble config.

It might even be valuable to provide some sort of visualization tool as part of this. Like one that shows a visual representation of the graph produced by Triton for the ensemble. This might be less trivial, but there are likely libraries that could make this easier.

@oandreeva-nv
Copy link
Contributor

I would be interested in implementing this

@rmccorm4
Copy link
Contributor

rmccorm4 commented Aug 8, 2023

I believe the developer of Netron is quite open to contribution/collaboration, and we use it often for ONNX and some TensorRT stuff - it might be worth adding a config as an acceptable format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants