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

[Task] Update model_pt.py to incorporate ignore_masking arg in TF4Rec model serving with Triton #1688

Closed
rnyak opened this issue Oct 5, 2022 · 1 comment · Fixed by NVIDIA-Merlin/Transformers4Rec#498
Assignees
Labels
bug Something isn't working enhancement New feature or request P0

Comments

@rnyak
Copy link
Contributor

rnyak commented Oct 5, 2022

What needs doing

A while go we added ignore_masking arg to ensure that when we do prediction with TF4Rec we do not apply masking, meaning we don’t want to mask any input in sequence, rather we use the whole sequence to predict the next item. We noticed that our model_pt.py does not incorporate ignore_masking=True in this line and we need to add it, as in here.

Additional context

@karlhigley recommends that if we want to avoid breaking other PyTorch models, seems like the cleanest approach would be to save T4R models for inference with an ignore_masking property set on them. Then the model's call/forward function could check the property and do the right thing without us having to add additional logic in the serving code. Since the Torchscript-based serving runs in a Triton back-end we don't control where we can't add code, we should be looking for solution that configures the model appropriately for serving before/while we save it.

@rnyak rnyak added bug Something isn't working P0 labels Oct 5, 2022
@rnyak rnyak added the enhancement New feature or request label Oct 5, 2022
@rnyak
Copy link
Contributor Author

rnyak commented Oct 5, 2022

@viswa-nvidia fyi. I set it to P0 because this is an important feature that is missing, and was noticed by our customer.

@rnyak rnyak changed the title [Task] Update model_pt.py to incorporate ignore_masking arg in TF4Rec [Task] Update model_pt.py to incorporate ignore_masking arg in TF4Rec model serving with Triton Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request P0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants