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.

Sign up for GitHub

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

Jump to bottom

Add logic to handle Python-based backends instead of platform handlers #303

Merged
merged 8 commits into from
Oct 7, 2023
Merged

Add logic to handle Python-based backends instead of platform handlers #303

merged 8 commits into from
Oct 7, 2023

Conversation

oandreeva-nv
Copy link
Contributor

@oandreeva-nv oandreeva-nv commented Sep 28, 2023 •
edited
Loading

Initial look and feel:
core changes here: triton-inference-server/core#269

open for discussions or names and logic

Currently tested on former pytorch platform handler model.py (works ootb simply by placing model.py to a dedicated backend folder) and vllm ( same, just small adjustmets to search model.json in model's directory via pb_utils.get_model_dir())

Next steps: remove platform_handlers from resources and build

@oandreeva-nv oandreeva-nv changed the title Added logic to handle python based backends instead of platform handlers [WIP] Added logic to handle python based backends instead of platform handlers Sep 28, 2023
@Tabrizian Tabrizian requested a review from krishung5 September 29, 2023 15:12
nnshah1
nnshah1 reviewed Sep 29, 2023
View reviewed changes
src/pb_stub.cc Outdated Show resolved Hide resolved
krishung5
krishung5 reviewed Oct 2, 2023
View reviewed changes
Copy link
Contributor

@krishung5 krishung5 left a comment

Choose a reason for hiding this comment

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

LGTM on how to handle the python based backends. I'd second Neelay's comment on the naming. I think using backend here makes sense to me, but open to any discussions.

@oandreeva-nv
Copy link
Contributor Author

I switched the name to python_runtime_model, this way, when it is not used, it is specified to DEFAULT, otherwise it is specified to location of model.py. Let me know if it makes sense

tanmayv25
tanmayv25 reviewed Oct 3, 2023
View reviewed changes
src/pb_stub.h Show resolved Hide resolved
src/pb_stub.cc Show resolved Hide resolved
src/python_be.cc Outdated Show resolved Hide resolved
src/python_be.cc Show resolved Hide resolved
src/python_be.cc Outdated Show resolved Hide resolved
@oandreeva-nv
Copy link
Contributor Author

I deleted platform handlers from cmake and corresponding repos. Didn't change README though

Tabrizian
Tabrizian reviewed Oct 5, 2023
View reviewed changes
src/pb_stub.cc Show resolved Hide resolved
src/python_be.cc Show resolved Hide resolved
src/python_be.cc Show resolved Hide resolved
src/resources/platform_handlers/tensorflow_savedmodel/model.py Show resolved Hide resolved
@dyastremsky dyastremsky changed the title [WIP] Added logic to handle python based backends instead of platform handlers Add logic to handle Python-based backends instead of platform handlers Oct 6, 2023
@dyastremsky dyastremsky merged commit cd68026 into main Oct 7, 2023
3 checks passed
@oandreeva-nv oandreeva-nv deleted the oandreeva_extract_platform_handlers_alt branch October 22, 2024 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dyastremsky dyastremsky dyastremsky left review comments

@Tabrizian Tabrizian Tabrizian left review comments

@tanmayv25 tanmayv25 tanmayv25 approved these changes

@GuanLuo GuanLuo Awaiting requested review from GuanLuo

@nnshah1 nnshah1 Awaiting requested review from nnshah1

@krishung5 krishung5 Awaiting requested review from krishung5

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@oandreeva-nv @Tabrizian @tanmayv25 @nnshah1 @krishung5 @dyastremsky