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

Quick start for Ranking - Include support for Triton inference #966

Merged
merged 31 commits into from
May 17, 2023

Conversation

rnyak
Copy link
Contributor

@rnyak rnyak commented May 4, 2023

Closes #919, closes #912, closes #913, closes #914

@rnyak rnyak added inference examples Adding new examples labels May 4, 2023
@rnyak rnyak added this to the Merlin 23.05 milestone May 4, 2023
@rnyak rnyak linked an issue May 4, 2023 that may be closed by this pull request
3 tasks
@github-actions
Copy link

github-actions bot commented May 4, 2023

Documentation preview

https://nvidia-merlin.github.io/Merlin/review/pr-966

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@rnyak rnyak changed the title [WIP] Quick start for Ranking - Include support for Triton inference Quick start for Ranking - Include support for Triton inference May 12, 2023
@rnyak rnyak requested a review from gabrielspmoreira May 12, 2023 16:19
Copy link
Member

@gabrielspmoreira gabrielspmoreira left a comment

Choose a reason for hiding this comment

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

Will continue the review Monday...

rnyak and others added 7 commits May 14, 2023 19:43
Co-authored-by: Gabriel Moreira <gmoreira@nvidia.com>
Co-authored-by: Gabriel Moreira <gmoreira@nvidia.com>
Co-authored-by: Gabriel Moreira <gmoreira@nvidia.com>
Co-authored-by: Gabriel Moreira <gmoreira@nvidia.com>
Co-authored-by: Gabriel Moreira <gmoreira@nvidia.com>
@bschifferer
Copy link
Contributor

rerun tests

@@ -0,0 +1,530 @@
{
Copy link
Member

@gabrielspmoreira gabrielspmoreira May 16, 2023

Choose a reason for hiding this comment

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

  • We could extract INPUT_FOLDER to a variable, so that users understand that path is something they could change.
  • I think that the /workspace is not a standard path inside the container, it is the one you mapped and provided to the preprocessing.py --output_path right? In ranking.md we set OUT_DATASET_PATH=/outputs/dataset

Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated that cell.

Copy link
Member

@gabrielspmoreira gabrielspmoreira left a comment

Choose a reason for hiding this comment

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

@rnyak I think we need to add a short section in the bottom of the main Quick-start for ranking page linking to the inference/README.md, like we do for the Hyperparameter tuning section.

Copy link
Member

@gabrielspmoreira gabrielspmoreira left a comment

Choose a reason for hiding this comment

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

@rnyak I think we need to add a short section in the bottom of the main Quick-start for ranking page linking to the inference/README.md, like we do for the Hyperparameter tuning section.

@edknv edknv merged commit 1781485 into main May 17, 2023
@edknv edknv deleted the quick_start_inf_triton branch May 17, 2023 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment