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

Pass model kwargs through to operator from_config methods #158

Merged
merged 2 commits into from
Aug 2, 2022

Conversation

oliverholworthy
Copy link
Member

Passing model kwargs (model_repository, model_name, model_version) through to operator from_config methods when used in an OperatorRunner.

Motivation

This enables operators to use the location of the model repository to load artifacts that may be required for the operator to run. For example, a serialized model saved alongside the model conifg (or in the model version directory).

Intended to be used by the Implict operator in #134 . We could also update the QueryFaiss operator to load the index from a relative path too instead of relying on a path in the model config. This would make the model repository self-contained.

@@ -18,7 +18,17 @@


class OperatorRunner:
def __init__(self, config, repository="./", version=1, kind=""):
Copy link
Member Author

Choose a reason for hiding this comment

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

removed kind since it doesn't appear to be used currently. Can be added back if required in future

def __init__(
self,
config,
*,
Copy link
Member Author

Choose a reason for hiding this comment

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

this forces the following parameters to be passed as keyword arguments.

@github-actions
Copy link

github-actions bot commented Aug 2, 2022

Documentation preview

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

@nvidia-merlin-bot
Copy link

Click to view CI Results
GitHub pull request #158 of commit 7d147b1708125d6f1f5586fb3a03ecd6f92a6172, no merge conflicts.
Running as SYSTEM
Setting status of 7d147b1708125d6f1f5586fb3a03ecd6f92a6172 to PENDING with url https://10.20.13.93:8080/job/merlin_systems/187/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/158/*:refs/remotes/origin/pr/158/* # timeout=10
 > git rev-parse 7d147b1708125d6f1f5586fb3a03ecd6f92a6172^{commit} # timeout=10
Checking out Revision 7d147b1708125d6f1f5586fb3a03ecd6f92a6172 (detached)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 7d147b1708125d6f1f5586fb3a03ecd6f92a6172 # timeout=10
Commit message: "Update tests for OperatorRunner to pass named keyword arguments"
 > git rev-list --no-walk f412ba518aa276b1ab0aabdc781bd501562d143f # timeout=10
[merlin_systems] $ /bin/bash /tmp/jenkins789547165856919159.sh
PYTHONPATH=:/usr/local/lib/python3.8/dist-packages/:/usr/local/hugectr/lib:/var/jenkins_home/workspace/merlin_systems/systems
============================= 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.6.1, xdist-2.5.0, forked-1.4.0, cov-3.0.0
collected 53 items

tests/unit/test_version.py . [ 1%]
tests/unit/examples/test_serving_ranking_models_with_merlin_systems.py . [ 3%]
[ 3%]
tests/unit/systems/test_ensemble.py .... [ 11%]
tests/unit/systems/test_ensemble_ops.py .. [ 15%]
tests/unit/systems/test_export.py . [ 16%]
tests/unit/systems/test_graph.py . [ 18%]
tests/unit/systems/test_inference_ops.py ... [ 24%]
tests/unit/systems/test_model_registry.py . [ 26%]
tests/unit/systems/test_op_runner.py .... [ 33%]
tests/unit/systems/test_tensorflow_inf_op.py .... [ 41%]
tests/unit/systems/dag/ops/test_softmax_sampling.py . [ 43%]
tests/unit/systems/fil/test_fil.py .......................... [ 92%]
tests/unit/systems/fil/test_forest.py .... [100%]

=============================== warnings summary ===============================
../../../../../usr/local/lib/python3.8/dist-packages/nvtabular/framework_utils/init.py:18
/usr/local/lib/python3.8/dist-packages/nvtabular/framework_utils/init.py:18: DeprecationWarning: The nvtabular.framework_utils module is being replaced by the Merlin Models library. Support for importing from nvtabular.framework_utils is deprecated, and will be removed in a future version. Please consider using the models and layers from Merlin Models instead.
warnings.warn(

tests/unit/examples/test_serving_ranking_models_with_merlin_systems.py: 1 warning
tests/unit/systems/test_ensemble.py: 2 warnings
tests/unit/systems/test_export.py: 1 warning
tests/unit/systems/test_inference_ops.py: 2 warnings
tests/unit/systems/test_op_runner.py: 4 warnings
/usr/local/lib/python3.8/dist-packages/cudf/core/frame.py:384: UserWarning: The deep parameter is ignored and is only included for pandas compatibility.
warnings.warn(

tests/unit/systems/test_export.py::test_export_run_ensemble_triton[tensorflow-parquet]
/var/jenkins_home/workspace/merlin_systems/systems/merlin/systems/triton/export.py:304: UserWarning: Column x is being generated by NVTabular workflow but is unused in test_name_tf model
warnings.warn(

tests/unit/systems/test_export.py::test_export_run_ensemble_triton[tensorflow-parquet]
/var/jenkins_home/workspace/merlin_systems/systems/merlin/systems/triton/export.py:304: UserWarning: Column y is being generated by NVTabular workflow but is unused in test_name_tf model
warnings.warn(

tests/unit/systems/test_export.py::test_export_run_ensemble_triton[tensorflow-parquet]
/var/jenkins_home/workspace/merlin_systems/systems/merlin/systems/triton/export.py:304: UserWarning: Column id is being generated by NVTabular workflow but is unused in test_name_tf model
warnings.warn(

tests/unit/systems/fil/test_fil.py::test_binary_classifier_default[sklearn_forest_classifier-get_model_params4]
tests/unit/systems/fil/test_fil.py::test_binary_classifier_with_proba[sklearn_forest_classifier-get_model_params4]
tests/unit/systems/fil/test_fil.py::test_multi_classifier[sklearn_forest_classifier-get_model_params4]
tests/unit/systems/fil/test_fil.py::test_regressor[sklearn_forest_regressor-get_model_params4]
tests/unit/systems/fil/test_fil.py::test_model_file[sklearn_forest_regressor-checkpoint.tl]
/usr/local/lib/python3.8/dist-packages/sklearn/utils/deprecation.py:103: FutureWarning: Attribute n_features_ was deprecated in version 1.0 and will be removed in 1.2. Use n_features_in_ instead.
warnings.warn(msg, category=FutureWarning)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================= 53 passed, 19 warnings in 239.97s (0:03:59) ==================
Performing Post build task...
Match found for : : True
Logical operation result is TRUE
Running script : #!/bin/bash
cd /var/jenkins_home/
CUDA_VISIBLE_DEVICES=1 python test_res_push.py "https://api.GitHub.com/repos/NVIDIA-Merlin/systems/issues/$ghprbPullId/comments" "/var/jenkins_home/jobs/$JOB_NAME/builds/$BUILD_NUMBER/log"
[merlin_systems] $ /bin/bash /tmp/jenkins13352577173853819069.sh

@oliverholworthy oliverholworthy self-assigned this Aug 2, 2022
@oliverholworthy oliverholworthy added the enhancement New feature or request label Aug 2, 2022
@karlhigley karlhigley merged commit 723fa61 into NVIDIA-Merlin:main Aug 2, 2022
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