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

[Bug]: Pytorch RunInference PredictionResult is a Dict #22240

Closed
yeandy opened this issue Jul 12, 2022 · 6 comments · Fixed by #23087 or #23152
Closed

[Bug]: Pytorch RunInference PredictionResult is a Dict #22240

yeandy opened this issue Jul 12, 2022 · 6 comments · Fixed by #23087 or #23152
Assignees
Labels
bug core done & done Issue has been reviewed after it was closed for verification, followups, etc. ml P2 python run-inference

Comments

@yeandy
Copy link
Contributor

yeandy commented Jul 12, 2022

What happened?

The return value of the forward call for many models is a dictionary with the predictions along with more metadata. i.e. Dict[str, Tensor]. However, RunInference currently expects outputs to be an Iterable[Any]. i.e. Iterable[Tensor] or Iterable[Dict[str, Tensor]]. So when RunInference zips the inputs with the predictions, the predictions iterate over the dictionary keys instead of the batch elements. So we end up with only the key name and actual prediction Tensors are discarded. This results in an unusable PredictionResult.

The PredictionResult challenge is a known quirk due to the current design/implementation of RunInference. We have a sample that does something similar by creating a wrapper class. We should support both tensors and dictionary of tensors. Currently, it only works for the former, and that is baked into the logic. We should figure out how to natively support results that are a dict type.

Issue Priority

Priority: 2

Issue Component

Component: sdk-py-core

@yeandy
Copy link
Contributor Author

yeandy commented Jul 12, 2022

.take-issue

@yeandy
Copy link
Contributor Author

yeandy commented Jul 15, 2022

Subtask of #22117

@damccorm
Copy link
Contributor

damccorm commented Sep 1, 2022

#22979 got created as an accidental duplicate. I'm closing that one, but copying over the description since I think it has some extra additional context:

Originally documented here in our RunInference Troubleshooting section:

For some models, the PredictionResults output might not include the correct predictions in the inferences field. This issue occurs when you use a model whose inferences return a dictionary that maps keys to predictions and other metadata. An example return type is Dict[str, Tensor].

The RunInference API currently expects outputs to be an Iterable[Any]. Example return types are Iterable[Tensor] or Iterable[Dict[str, Tensor]]. When RunInference zips the inputs with the predictions, the predictions iterate over the dictionary keys instead of the batch elements. The result is that the key name is preserved but the prediction tensors are discarded.

To work with the current RunInference implementation, you can create a wrapper class that overrides the model(input) call. In PyTorch, for example, your wrapper would override the forward() function and return an output with the appropriate format of List[Dict[str, torch.Tensor]].

@yeandy
Copy link
Contributor Author

yeandy commented Sep 9, 2022

Thanks @damccorm! We should also remove (or at least clarify) the section on this issue from our Troubleshooting guide. I had intended to make this comment in the PR yesterday, but forgot, and just remembered. 😄

@damccorm
Copy link
Contributor

damccorm commented Sep 9, 2022

That's a good callout - I'll reopen here. Since this fix won't be released for ~2 months (it missed the 2.42 cut), we should probably hold off on doing that until then.

@damccorm
Copy link
Contributor

damccorm commented Sep 9, 2022

#23152 will close this once its merged (after the 2.43 release)

@damccorm damccorm added the ml label Oct 4, 2022
@github-actions github-actions bot added this to the 2.44.0 Release milestone Nov 18, 2022
@damccorm damccorm added the done & done Issue has been reviewed after it was closed for verification, followups, etc. label Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug core done & done Issue has been reviewed after it was closed for verification, followups, etc. ml P2 python run-inference
Projects
None yet
2 participants