-
Notifications
You must be signed in to change notification settings - Fork 30
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
Fixes Merlin e2e example #117
Conversation
Click to view CI ResultsGitHub pull request #117 of commit 0f9fa18556f24ec9fc194d52cc3733a31f531826, no merge conflicts. Running as SYSTEM Setting status of 0f9fa18556f24ec9fc194d52cc3733a31f531826 to PENDING with url https://10.20.13.93:8080/job/merlin_systems/69/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/117/*:refs/remotes/origin/pr/117/* # timeout=10 > git rev-parse 0f9fa18556f24ec9fc194d52cc3733a31f531826^{commit} # timeout=10 Checking out Revision 0f9fa18556f24ec9fc194d52cc3733a31f531826 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 0f9fa18556f24ec9fc194d52cc3733a31f531826 # timeout=10 Commit message: "fixes to ensure use of correct keys from tf models" > git rev-list --no-walk 2a413f6e0993969f2aeab999c9e7cf968b799e7b # timeout=10 [merlin_systems] $ /bin/bash /tmp/jenkins14260605814698537897.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 6 items / 5 errors / 1 skipped |
Click to view CI ResultsGitHub pull request #117 of commit a25c7e90bc2adad53a75499fdbdeb14059b5607e, no merge conflicts. Running as SYSTEM Setting status of a25c7e90bc2adad53a75499fdbdeb14059b5607e to PENDING with url https://10.20.13.93:8080/job/merlin_systems/70/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/117/*:refs/remotes/origin/pr/117/* # timeout=10 > git rev-parse a25c7e90bc2adad53a75499fdbdeb14059b5607e^{commit} # timeout=10 Checking out Revision a25c7e90bc2adad53a75499fdbdeb14059b5607e (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f a25c7e90bc2adad53a75499fdbdeb14059b5607e # timeout=10 Commit message: "Merge branch 'main' into fix_model_outputnames" > git rev-list --no-walk 0f9fa18556f24ec9fc194d52cc3733a31f531826 # timeout=10 [merlin_systems] $ /bin/bash /tmp/jenkins12352964776961901369.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 6 items / 5 errors / 1 skipped |
Documentation preview |
merlin/systems/dag/ops/tensorflow.py
Outdated
@@ -145,35 +144,37 @@ def _export_model(self, model, name, output_path, version=1): | |||
name=name, backend="tensorflow", platform="tensorflow_savedmodel" | |||
) | |||
|
|||
inputs, outputs = model.inputs, model.outputs | |||
# inputs, outputs = model.inputs, [model.outputs] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets delete old code rather than comment out
# inputs, outputs = model.inputs, [model.outputs] |
merlin/systems/dag/ops/tensorflow.py
Outdated
|
||
config.parameters["TF_GRAPH_TAG"].string_value = "serve" | ||
config.parameters["TF_SIGNATURE_DEF"].string_value = "serving_default" | ||
|
||
for col in inputs: | ||
for col, col_name in zip(inputs, input_col_names): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about instead of zipping the keys/values of the default_signature.structured_input_signature[1]
- we just iterate over the items? It will be cleaner and less susceptible to bugs in the future
for col, col_name in zip(inputs, input_col_names): | |
for col, col_name in default_signature.structured_input_signature[1].items(): |
(and then don't create the 'input_col_names' and 'inputs' on lines 157/160
merlin/systems/dag/ops/tensorflow.py
Outdated
) | ||
) | ||
|
||
for col in outputs: | ||
for col, col_name in zip(outputs, output_col_names): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe the same thing as for inputs?
for col, col_name in zip(outputs, output_col_names): | |
for col, col_name in default_signature.structured_outputs.items(): |
Click to view CI ResultsGitHub pull request #117 of commit 38367f43dc98741eb95c39514af669c0e111ffc8, no merge conflicts. Running as SYSTEM Setting status of 38367f43dc98741eb95c39514af669c0e111ffc8 to PENDING with url https://10.20.13.93:8080/job/merlin_systems/74/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/117/*:refs/remotes/origin/pr/117/* # timeout=10 > git rev-parse 38367f43dc98741eb95c39514af669c0e111ffc8^{commit} # timeout=10 Checking out Revision 38367f43dc98741eb95c39514af669c0e111ffc8 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 38367f43dc98741eb95c39514af669c0e111ffc8 # timeout=10 Commit message: "Merge branch 'fix_model_outputnames' of https://github.com/jperez999/systems-1 into fix_model_outputnames" > git rev-list --no-walk 2a413f6e0993969f2aeab999c9e7cf968b799e7b # timeout=10 [merlin_systems] $ /bin/bash /tmp/jenkins7719179364039111327.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 / 1 skipped |
Click to view CI ResultsGitHub pull request #117 of commit 7331d722ebb1a84048eca4183b9bfe4f6994d76e, no merge conflicts. Running as SYSTEM Setting status of 7331d722ebb1a84048eca4183b9bfe4f6994d76e to PENDING with url https://10.20.13.93:8080/job/merlin_systems/75/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/117/*:refs/remotes/origin/pr/117/* # timeout=10 > git rev-parse 7331d722ebb1a84048eca4183b9bfe4f6994d76e^{commit} # timeout=10 Checking out Revision 7331d722ebb1a84048eca4183b9bfe4f6994d76e (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 7331d722ebb1a84048eca4183b9bfe4f6994d76e # timeout=10 Commit message: "Merge branch 'main' into fix_model_outputnames" > git rev-list --no-walk 38367f43dc98741eb95c39514af669c0e111ffc8 # timeout=10 [merlin_systems] $ /bin/bash /tmp/jenkins9215977444994546154.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 / 1 skipped |
rerun tests |
1 similar comment
rerun tests |
Click to view CI ResultsGitHub pull request #117 of commit 1e1226f81fe02bbdf1ede45ff8401ffaedbb01e0, no merge conflicts. Running as SYSTEM Setting status of 1e1226f81fe02bbdf1ede45ff8401ffaedbb01e0 to PENDING with url https://10.20.13.93:8080/job/merlin_systems/76/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/117/*:refs/remotes/origin/pr/117/* # timeout=10 > git rev-parse 1e1226f81fe02bbdf1ede45ff8401ffaedbb01e0^{commit} # timeout=10 Checking out Revision 1e1226f81fe02bbdf1ede45ff8401ffaedbb01e0 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 1e1226f81fe02bbdf1ede45ff8401ffaedbb01e0 # timeout=10 Commit message: "Merge branch 'fix_model_outputnames' of https://github.com/jperez999/systems-1 into fix_model_outputnames" > git rev-list --no-walk 7331d722ebb1a84048eca4183b9bfe4f6994d76e # timeout=10 [merlin_systems] $ /bin/bash /tmp/jenkins3091504900765628636.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 / 1 skipped |
@@ -91,7 +91,7 @@ def export(self, path, input_schema, output_schema, node_id=None, version=1): | |||
modified_workflow, | |||
node_name, | |||
node_export_path, | |||
backend="python", | |||
backend="nvtabular", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this work on the containers? I saw this PR go through, and I'm wondering if the NVT backend is still available https://github.com/NVIDIA-Merlin/Merlin/pull/378/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats why I need the other PR in merlin to go through also... need to recook
ensures we pull correct column information from Tensorflow models.