pytorch_backend icon indicating copy to clipboard operation
pytorch_backend copied to clipboard

Support calling custom method names on PyTorch module

Open iceychris opened this issue 2 years ago • 5 comments

  • [x] basic support
  • [x] test with dict inputs
  • [x] test with list inputs
  • [x] test default method name ("forward")
  • [x] test custom method name
  • [x] format

iceychris avatar Apr 08 '23 14:04 iceychris

@dyastremsky @tanmayv25 Mind looking into this? This PR should be ready.

I tested locally by building & running tritonserver with the modified backend code (list/dict inputs, new flag present/not present in config).

iceychris avatar Apr 11 '23 10:04 iceychris

Thanks for submitting this PR and tagging us, Christian! We'll need a Contributor License Agreement (CLA) here to merge these changes. You can see how to submit it here. That's also got directions on formatting this code to be aligned with the rest of the repo.

In addition to reviewing your ticket, the other thing we'll need to look into once we get your CLA is adding tests. We appreciate you manually testing, we do need automated tests to ensure this passes and doesn't break in the future. You can see how this is done in the server qa folder here. The model generation is done in the qa's common folder. If you're unable to add the tests, we may be able to do so, though then we'll need time to get to this ticket.

the-david-oy avatar Apr 11 '23 15:04 the-david-oy

@iceychris Thanks for your contribution! We are still waiting from the signed CLA from you to proceed with it further. Please let me know once it is completed. I can work on adding test cases for this on our CI if you'd like.

tanmayv25 avatar Apr 18 '23 19:04 tanmayv25

@iceychris Can you let this PR finish merging? This is a very important feature.

lifeiteng avatar Oct 04 '23 13:10 lifeiteng

@tanmayv25 I signed the CLA along with the commit and rebased onto main. Haven't had a chance to look into setting up proper testing though. Can you take it from here?

iceychris avatar Jan 29 '24 16:01 iceychris