Implicit state management #103
Conversation
|
Really appreciate your contribution to the Triton project! Could you please sign the CLA as instructed here? |
|
Sorry for the delay on this, that should be sent over now |
|
Just wanted to bump this again, have you received the CLA? I'm aware I need to add some tests from the qa folder in the main server repo, could you point me to the ones I should be looking at? |
|
Hi @jamied157, I can confirm that we've received the CLA. You need to It might be a bit difficult to get started with testing infra. Feel free to give it a shot and if you run into issues I can take care of testing. |
|
Hi again, I haven't been able to have a look at the tests yet (and it might take me a while), so feel free to get started if you can. Otherwise I'll take a look when I've got the time. |
src/libtorch.cc
Outdated
| @@ -35,6 +36,7 @@ | |||
| #include "triton/backend/backend_output_responder.h" | |||
| #include "triton/common/nvtx.h" | |||
| #include "triton/core/tritonbackend.h" | |||
| #include "triton/core/tritonserver.h" | |||
There was a problem hiding this comment.
I don't think this import is required.
| // can have intersection with the outputs section of the model. If an output | ||
| // is specified both in the output section and state section, it indicates | ||
| // that the backend must return the output state to the client too. | ||
| std::map<std::string, std::pair<int64_t, int64_t>> model_outputs_; |
There was a problem hiding this comment.
Can you mention in the comment that the first pair is the model output index and second pair is the state index. -1 will be used if either of them are not required.
src/libtorch.cc
Outdated
| input_index_map_[io_name] = | ||
| std::distance(allowed_inputs.begin(), itr); | ||
| } | ||
| return; |
There was a problem hiding this comment.
I think you should return error on the else condition that input is not valid.
There was a problem hiding this comment.
I think this comment is outdated as this change went in as part of a different MR: https://github.com/triton-inference-server/pytorch_backend/pull/98/files#diff-859dcc1d94f2d13b0d19e7a14af430cd18eb66395a6f69f2d4886854319c4467R795
src/libtorch.cc
Outdated
| .c_str()); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
I think this is gone
src/libtorch.cc
Outdated
| @@ -25,6 +25,7 @@ | |||
| // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | |||
|
|
|||
| #include <stdint.h> | |||
| #include <cstdint> | |||
jamied157
force-pushed
the
state_management
branch
from
f4533fb to
cc0657e
Compare
Tabrizian
force-pushed
the
state_management
branch
from
30a5df0 to
b290196
Compare
|
Thanks for continuing to work on this! |
Addresses triton-inference-server/server#5609
I tried to copy the onnxruntime backend as much as possible. To work with how PyTorch treats input and output names, I've tried to only allow the "INPUT__X/OUTPUT__X" naming convention when a state field is defined.