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

Enable cpp code path for Categorify ops #389

Merged
merged 9 commits into from
Jun 11, 2024

Conversation

rjzamora
Copy link
Contributor

Rolls back one of the changes in #354 to improve inference performance of Categorify.

@rjzamora rjzamora marked this pull request as ready for review April 11, 2024 16:13
@rjzamora rjzamora self-assigned this Apr 11, 2024
@oliverholworthy oliverholworthy added the enhancement New feature or request label Apr 23, 2024
Copy link

Documentation preview

https://nvidia-merlin.github.io/systems/review/pr-389

@oliverholworthy
Copy link
Member

the docs related errors are now resolved.

There's two remaining things for this MR:

@rjzamora
Copy link
Contributor Author

There's two remaining things for this MR: ...

Thanks so much @oliverholworthy ! Sorry, I missed this earlier.

@oliverholworthy
Copy link
Member

Following the merge of NVIDIA-Merlin/NVTabular#1874 we're now getting a new error in the tests

RuntimeError: Error: <class 'ValueError'> - Unknown column for CategorifyTransform x__values

@rjzamora
Copy link
Contributor Author

I haven't had the time to understand the test failure well enough to work through a long-term fix. However, my last commit roles back the decision to use the cpp code path for "Categorify" by default. Instead, when the WorkflowRunner is initialized, the value of the NVT_CPP_OPS environment variable will dictate which NVT ops the cpp code path will be used for.

For example, you can enable the cpp version of Categorify by deploying a triton model as follows:

NVT_CPP_OPS="Categorify" tritonserver --model-repository=/my-model/

@jperez999
Copy link
Collaborator

jperez999 commented Jun 7, 2024

This is failing right now on some package mismatches right now. A solution is to update the tritonclient package to 2.30.0. Working on that solution now. Then we should be able to pick up the newest version of CI container to fix the triton errors.

@jperez999 jperez999 mentioned this pull request Jun 11, 2024
@jperez999 jperez999 merged commit a19d311 into NVIDIA-Merlin:main Jun 11, 2024
9 checks passed
@rjzamora rjzamora deleted the enable-cpp-categorify branch June 11, 2024 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants