Add Forest Inference Operator #118
Conversation
Click to view CI ResultsGitHub pull request #118 of commit e0d9a9d078fa5d8d3c6dd1ba115ecdc5ed6c096a, no merge conflicts.
Running as SYSTEM
Setting status of e0d9a9d078fa5d8d3c6dd1ba115ecdc5ed6c096a to PENDING with url https://10.20.13.93:8080/job/merlin_systems/77/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_systems
using credential fce1c729-5d7c-48e8-90cb-b0c314b1076e
> git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
> git config remote.origin.url https://github.com/NVIDIA-Merlin/systems # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/systems
> git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems user + githubtoken
> git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/systems +refs/pull/118/*:refs/remotes/origin/pr/118/* # timeout=10
> git rev-parse e0d9a9d078fa5d8d3c6dd1ba115ecdc5ed6c096a^{commit} # timeout=10
Checking out Revision e0d9a9d078fa5d8d3c6dd1ba115ecdc5ed6c096a (detached)
> git config core.sparsecheckout # timeout=10
> git checkout -f e0d9a9d078fa5d8d3c6dd1ba115ecdc5ed6c096a # timeout=10
Commit message: "Add Forest class for running inference with forest models"
> git rev-list --no-walk 1e1226f81fe02bbdf1ede45ff8401ffaedbb01e0 # timeout=10
[merlin_systems] $ /bin/bash /tmp/jenkins15805946083825117376.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_systems/systems, configfile: pyproject.toml
plugins: anyio-3.5.0, xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 18 items / 2 skipped
|
Documentation preview |
Click to view CI ResultsGitHub pull request #118 of commit f5234219b6ec6bb083d8420775a96e35a764ed3e, no merge conflicts.
Running as SYSTEM
Setting status of f5234219b6ec6bb083d8420775a96e35a764ed3e to PENDING with url https://10.20.13.93:8080/job/merlin_systems/78/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_systems
using credential fce1c729-5d7c-48e8-90cb-b0c314b1076e
> git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
> git config remote.origin.url https://github.com/NVIDIA-Merlin/systems # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/systems
> git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems user + githubtoken
> git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/systems +refs/pull/118/*:refs/remotes/origin/pr/118/* # timeout=10
> git rev-parse f5234219b6ec6bb083d8420775a96e35a764ed3e^{commit} # timeout=10
Checking out Revision f5234219b6ec6bb083d8420775a96e35a764ed3e (detached)
> git config core.sparsecheckout # timeout=10
> git checkout -f f5234219b6ec6bb083d8420775a96e35a764ed3e # timeout=10
Commit message: "Add test for Ensemble with NVTabular Workflow and Forest Operator"
> git rev-list --no-walk e0d9a9d078fa5d8d3c6dd1ba115ecdc5ed6c096a # timeout=10
[merlin_systems] $ /bin/bash /tmp/jenkins17896677633240590267.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_systems/systems, configfile: pyproject.toml
plugins: anyio-3.5.0, xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 18 items / 2 skipped
|
oliverholworthy
force-pushed
the
ops-fil-wrapper
branch
from
f523421
to
36a7cdf
Compare
Click to view CI ResultsGitHub pull request #118 of commit 36a7cdf5b8766f0007d62fedc6fee03097190227, no merge conflicts.
Running as SYSTEM
Setting status of 36a7cdf5b8766f0007d62fedc6fee03097190227 to PENDING with url https://10.20.13.93:8080/job/merlin_systems/79/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_systems
using credential fce1c729-5d7c-48e8-90cb-b0c314b1076e
> git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
> git config remote.origin.url https://github.com/NVIDIA-Merlin/systems # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/systems
> git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems user + githubtoken
> git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/systems +refs/pull/118/*:refs/remotes/origin/pr/118/* # timeout=10
> git rev-parse 36a7cdf5b8766f0007d62fedc6fee03097190227^{commit} # timeout=10
Checking out Revision 36a7cdf5b8766f0007d62fedc6fee03097190227 (detached)
> git config core.sparsecheckout # timeout=10
> git checkout -f 36a7cdf5b8766f0007d62fedc6fee03097190227 # timeout=10
Commit message: "Add test for Ensemble with NVTabular Workflow and Forest Operator"
> git rev-list --no-walk f5234219b6ec6bb083d8420775a96e35a764ed3e # timeout=10
[merlin_systems] $ /bin/bash /tmp/jenkins2740923852274120150.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_systems/systems, configfile: pyproject.toml
plugins: anyio-3.5.0, xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 18 items / 2 skipped
|
There was a problem hiding this comment.
This looks really good and solves a long-standing problem we've had re: back-ends with weird input formats vs accurate schemas that pass validation ❤️
Looks like there are some test failures but I think they're minor "typo in the assertion" kinds of things, so I'm optimistically approving it and let's get it in As Soon As Reasonable Without Overly Exerting Ourselves (ASARWOEO.) 😺
merlin/systems/dag/ops/fil.py
Outdated
| pb_utils = None | ||
|
|
||
|
|
||
| class Forest(PipelineableInferenceOperator): |
There was a problem hiding this comment.
This is less an issue with this PR and more an issue for the library generally, which I don't think should block this PR but is probably a worthwhile point of discussion on which I'm open to suggestions:
We should probably come up with a consistent naming scheme for the operators that make actual model predictions in separate back-ends (like FIL and Tensorflow.) Originally I was thinking to make the convention FrameworkNamePredict, which I thought read pretty nicely in the operator graph definition. On second thought, I realized it might be nice for the prediction operators to be conceptually and alphabetically grouped together, so we changed the name to PredictTensorflow, which would imply this operator might be called PredictForest or PredictFIL. I'm not entirely happy with either convention though, so open to suggestions for how we might do this in a consistent way.
There was a problem hiding this comment.
(Also, as a point of order for interpreting my PR comments: I mean this as literally as you can possibly take it—and I usually do. This is not a polite way of saying something else; I am not making a veiled suggestion for changing the code herein. I honestly don't know what to do here, and could be persuaded to go in any direction someone makes a halfway compelling case for. I just think we should probably, at some point, not necessarily now or in this PR, figure out how to name these operators consistently but I can't myself make a halfway compelling case for any particular convention, so...I don't really have a suggestion here, just an issue to raise for your consideration and future pondering. Happy to address this someday, when we think of something smarter than I have so far.)
There was a problem hiding this comment.
For consistency with the current operators, I've updated to use the name PredictForest. There seems to be a similar pattern of <Type of Operatior><Type Of Data or Model> (QueryFaiss, QueryFeast, PredictTensorflow, FilterCandidates, UnrollFeatures, TransformWorkflow)
|
|
||
| config_path = tmpdir / "1_fil" / "config.pbtxt" | ||
| parsed_config = read_config(config_path) | ||
| assert parsed_config.name == "0_fil" |
There was a problem hiding this comment.
This should maybe also be 1_fil, but I'm honestly not 100% sure how (or if) we should account for this op in the ensemble operator id numbering. I think it could be problematic to have two operators with the same id integer, but I don't think this code actually affects the mapping of ids to operators, so maybe aligning the prefix integer with the id of the operator it belongs to makes sense here? Assuming this doesn't break anything, I have no objections to this convention, I'm just thinking out loud here.
There was a problem hiding this comment.
Yes, I think having the fIl model prefix id match the PredictForest id as it currently does (aside from this test typo) makes sense because they’re part of the same operator conceptually, from the perspective of the user of the library. And it should avoid collisions if there are more than one in the ensemble.
Click to view CI ResultsGitHub pull request #118 of commit 46e82485690f300ba3371f926be021b04f678ff0, no merge conflicts.
Running as SYSTEM
Setting status of 46e82485690f300ba3371f926be021b04f678ff0 to PENDING with url https://10.20.13.93:8080/job/merlin_systems/80/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_systems
using credential fce1c729-5d7c-48e8-90cb-b0c314b1076e
> git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
> git config remote.origin.url https://github.com/NVIDIA-Merlin/systems # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/systems
> git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems user + githubtoken
> git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/systems +refs/pull/118/*:refs/remotes/origin/pr/118/* # timeout=10
> git rev-parse 46e82485690f300ba3371f926be021b04f678ff0^{commit} # timeout=10
Checking out Revision 46e82485690f300ba3371f926be021b04f678ff0 (detached)
> git config core.sparsecheckout # timeout=10
> git checkout -f 46e82485690f300ba3371f926be021b04f678ff0 # timeout=10
Commit message: "Correct config name in forest test"
> git rev-list --no-walk 36a7cdf5b8766f0007d62fedc6fee03097190227 # timeout=10
[merlin_systems] $ /bin/bash /tmp/jenkins8461255176555227623.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_systems/systems, configfile: pyproject.toml
plugins: anyio-3.5.0, xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 18 items / 2 skipped
|
Click to view CI ResultsGitHub pull request #118 of commit 604b232a994670f96e6948a684a3d22612f17909, no merge conflicts.
Running as SYSTEM
Setting status of 604b232a994670f96e6948a684a3d22612f17909 to PENDING with url https://10.20.13.93:8080/job/merlin_systems/81/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_systems
using credential fce1c729-5d7c-48e8-90cb-b0c314b1076e
> git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
> git config remote.origin.url https://github.com/NVIDIA-Merlin/systems # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/systems
> git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems user + githubtoken
> git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/systems +refs/pull/118/*:refs/remotes/origin/pr/118/* # timeout=10
> git rev-parse 604b232a994670f96e6948a684a3d22612f17909^{commit} # timeout=10
Checking out Revision 604b232a994670f96e6948a684a3d22612f17909 (detached)
> git config core.sparsecheckout # timeout=10
> git checkout -f 604b232a994670f96e6948a684a3d22612f17909 # timeout=10
Commit message: "Rename Forest PredictForest and update docstring."
> git rev-list --no-walk 46e82485690f300ba3371f926be021b04f678ff0 # timeout=10
[merlin_systems] $ /bin/bash /tmp/jenkins12417168055295635629.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_systems/systems, configfile: pyproject.toml
plugins: anyio-3.5.0, xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 18 items / 2 skipped
|
Click to view CI ResultsGitHub pull request #118 of commit 5738256b1766f6aea3d2e9a2d48cf040512d73b7, no merge conflicts.
Running as SYSTEM
Setting status of 5738256b1766f6aea3d2e9a2d48cf040512d73b7 to PENDING with url https://10.20.13.93:8080/job/merlin_systems/82/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_systems
using credential fce1c729-5d7c-48e8-90cb-b0c314b1076e
> git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
> git config remote.origin.url https://github.com/NVIDIA-Merlin/systems # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/systems
> git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems user + githubtoken
> git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/systems +refs/pull/118/*:refs/remotes/origin/pr/118/* # timeout=10
> git rev-parse 5738256b1766f6aea3d2e9a2d48cf040512d73b7^{commit} # timeout=10
Checking out Revision 5738256b1766f6aea3d2e9a2d48cf040512d73b7 (detached)
> git config core.sparsecheckout # timeout=10
> git checkout -f 5738256b1766f6aea3d2e9a2d48cf040512d73b7 # timeout=10
Commit message: "Correct name of directory in tests"
> git rev-list --no-walk 604b232a994670f96e6948a684a3d22612f17909 # timeout=10
[merlin_systems] $ /bin/bash /tmp/jenkins9073330330584653846.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_systems/systems, configfile: pyproject.toml
plugins: anyio-3.5.0, xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 18 items / 2 skipped
|
oliverholworthy
force-pushed
the
ops-fil-wrapper
branch
from
5738256
to
30c6878
Compare
Click to view CI ResultsGitHub pull request #118 of commit 30c6878d37785647b338bcd79bb78ace64f8f84a, no merge conflicts.
Running as SYSTEM
Setting status of 30c6878d37785647b338bcd79bb78ace64f8f84a to PENDING with url https://10.20.13.93:8080/job/merlin_systems/83/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_systems
using credential fce1c729-5d7c-48e8-90cb-b0c314b1076e
> git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
> git config remote.origin.url https://github.com/NVIDIA-Merlin/systems # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/systems
> git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems user + githubtoken
> git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/systems +refs/pull/118/*:refs/remotes/origin/pr/118/* # timeout=10
> git rev-parse 30c6878d37785647b338bcd79bb78ace64f8f84a^{commit} # timeout=10
Checking out Revision 30c6878d37785647b338bcd79bb78ace64f8f84a (detached)
> git config core.sparsecheckout # timeout=10
> git checkout -f 30c6878d37785647b338bcd79bb78ace64f8f84a # timeout=10
Commit message: "Correct name of directory in tests"
> git rev-list --no-walk 5738256b1766f6aea3d2e9a2d48cf040512d73b7 # timeout=10
[merlin_systems] $ /bin/bash /tmp/jenkins16655649153270021424.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_systems/systems, configfile: pyproject.toml
plugins: anyio-3.5.0, xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 18 items / 2 skipped
|
Click to view CI ResultsGitHub pull request #118 of commit 816abb74d0231d0a8f9ced62f48a2b3017ee5ded, no merge conflicts.
Running as SYSTEM
Setting status of 816abb74d0231d0a8f9ced62f48a2b3017ee5ded to PENDING with url https://10.20.13.93:8080/job/merlin_systems/87/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_systems
using credential fce1c729-5d7c-48e8-90cb-b0c314b1076e
> git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
> git config remote.origin.url https://github.com/NVIDIA-Merlin/systems # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/systems
> git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems user + githubtoken
> git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/systems +refs/pull/118/*:refs/remotes/origin/pr/118/* # timeout=10
> git rev-parse 816abb74d0231d0a8f9ced62f48a2b3017ee5ded^{commit} # timeout=10
Checking out Revision 816abb74d0231d0a8f9ced62f48a2b3017ee5ded (detached)
> git config core.sparsecheckout # timeout=10
> git checkout -f 816abb74d0231d0a8f9ced62f48a2b3017ee5ded # timeout=10
Commit message: "Merge branch 'main' into ops-fil-wrapper"
> git rev-list --no-walk 93dfe203bf03de2d69ce42c488562e7e3107a367 # timeout=10
[merlin_systems] $ /bin/bash /tmp/jenkins10462710633549038945.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_systems/systems, configfile: pyproject.toml
plugins: anyio-3.5.0, xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 17 items / 2 skipped
|
Click to view CI ResultsGitHub pull request #118 of commit eaf7e3253cdb74f57a417c14ff7642a272ba182a, no merge conflicts.
Running as SYSTEM
Setting status of eaf7e3253cdb74f57a417c14ff7642a272ba182a to PENDING with url https://10.20.13.93:8080/job/merlin_systems/88/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_systems
using credential fce1c729-5d7c-48e8-90cb-b0c314b1076e
> git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
> git config remote.origin.url https://github.com/NVIDIA-Merlin/systems # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/systems
> git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems user + githubtoken
> git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/systems +refs/pull/118/*:refs/remotes/origin/pr/118/* # timeout=10
> git rev-parse eaf7e3253cdb74f57a417c14ff7642a272ba182a^{commit} # timeout=10
Checking out Revision eaf7e3253cdb74f57a417c14ff7642a272ba182a (detached)
> git config core.sparsecheckout # timeout=10
> git checkout -f eaf7e3253cdb74f57a417c14ff7642a272ba182a # timeout=10
Commit message: "Merge branch 'main' into ops-fil-wrapper"
> git rev-list --no-walk 816abb74d0231d0a8f9ced62f48a2b3017ee5ded # timeout=10
[merlin_systems] $ /bin/bash /tmp/jenkins18123301131867031504.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_systems/systems, configfile: pyproject.toml
plugins: anyio-3.5.0, xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 17 items / 2 skipped
|
Click to view CI ResultsGitHub pull request #118 of commit 96e6d80c37f296f7c8ef59118fcc679d9dc3ec8a, no merge conflicts.
Running as SYSTEM
Setting status of 96e6d80c37f296f7c8ef59118fcc679d9dc3ec8a to PENDING with url https://10.20.13.93:8080/job/merlin_systems/89/console and message: 'Pending'
Using context: Jenkins
Building on master in workspace /var/jenkins_home/workspace/merlin_systems
using credential fce1c729-5d7c-48e8-90cb-b0c314b1076e
> git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
> git config remote.origin.url https://github.com/NVIDIA-Merlin/systems # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/systems
> git --version # timeout=10
using GIT_ASKPASS to set credentials login for merlin-systems user + githubtoken
> git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/systems +refs/pull/118/*:refs/remotes/origin/pr/118/* # timeout=10
> git rev-parse 96e6d80c37f296f7c8ef59118fcc679d9dc3ec8a^{commit} # timeout=10
Checking out Revision 96e6d80c37f296f7c8ef59118fcc679d9dc3ec8a (detached)
> git config core.sparsecheckout # timeout=10
> git checkout -f 96e6d80c37f296f7c8ef59118fcc679d9dc3ec8a # timeout=10
Commit message: "Correct backend assertion for transform op in forest ensemble test"
> git rev-list --no-walk eaf7e3253cdb74f57a417c14ff7642a272ba182a # timeout=10
[merlin_systems] $ /bin/bash /tmp/jenkins16938884123612280413.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.2, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/merlin_systems/systems, configfile: pyproject.toml
plugins: anyio-3.5.0, xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 17 items / 2 skipped
|
|
@oliverholworthy what is We have input_schema and feature_names. Is this redundant? What is the difference? |
|
I would like to second what @bschifferer raised -- passing the schema into @oliverholworthy I am working on #136 and ran into some issues. To figure out what is going on, I grabbed the code for I then attempt to start the triton inference with Could I please ask you for help? Do you know what might be going on there? |
|
I was able to figure out what is going on there, no further assistance on this would be needed. |
This may have been discussed in Slack. The reason the input schema is required as an argument to the operator is because the model objects don't have enough information to determine the schema required to setup the operator correctly. Tensorflow models have a way to inspect the signature including names and dtypes of the inputs expected. There was some discussion about coming up with a serialization format that will allow us to figure this out from any model (built with Merlin Models) NVIDIA-Merlin/Merlin#489 |
Add
PredictForestInference Operator. Wraps the FIL operator added in #110 And uses the python backend and Business Logic Scripting to make requests to the FIL model.Exporting the Forest operator results in two Triton models:
Example
Note on GPU usage
I encountered an issue when using GPU with the FIL op + the PredictForest wrapper op. I have been receiving the following error:
A work-around I found was to set the instance group to CPU