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

Set ignore_masking to True by default #498

Merged
merged 2 commits into from
Oct 10, 2022
Merged

Set ignore_masking to True by default #498

merged 2 commits into from
Oct 10, 2022

Conversation

sararb
Copy link
Contributor

@sararb sararb commented Oct 5, 2022

Fixes NVIDIA-Merlin/NVTabular#1688

  • This is a quick fix to set ignore_masking to True in the model's forward method.
  • The motivation behind this fix is that during prediction/inference, we always want to use the whole input sequence without any masking.

@sararb sararb requested a review from rnyak October 5, 2022 18:05
@sararb sararb self-assigned this Oct 5, 2022
@sararb sararb added the enhancement New feature or request label Oct 5, 2022
@nvidia-merlin-bot
Copy link

Click to view CI Results
GitHub pull request #498 of commit 10716bfb800e562c74945a34acaa4cbddaad1552, no merge conflicts.
Running as SYSTEM
Setting status of 10716bfb800e562c74945a34acaa4cbddaad1552 to PENDING with url http://10.20.17.181:8080/job/transformers4rec_tests/202/ and message: 'Build started for merge commit.'
Using context: Jenkins Unit Test Run
Building on master in workspace /var/jenkins_home/workspace/transformers4rec_tests
using credential nvidia-merlin-bot
Cloning the remote Git repository
Cloning repository https://github.com/NVIDIA-Merlin/Transformers4Rec.git
 > git init /var/jenkins_home/workspace/transformers4rec_tests/transformers4rec # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/Transformers4Rec.git
 > git --version # timeout=10
using GIT_ASKPASS to set credentials This is the bot credentials for our CI/CD
 > git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/Transformers4Rec.git +refs/heads/*:refs/remotes/origin/* # timeout=10
 > git config remote.origin.url https://github.com/NVIDIA-Merlin/Transformers4Rec.git # timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # timeout=10
 > git config remote.origin.url https://github.com/NVIDIA-Merlin/Transformers4Rec.git # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/Transformers4Rec.git
using GIT_ASKPASS to set credentials This is the bot credentials for our CI/CD
 > git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/Transformers4Rec.git +refs/pull/498/*:refs/remotes/origin/pr/498/* # timeout=10
 > git rev-parse 10716bfb800e562c74945a34acaa4cbddaad1552^{commit} # timeout=10
Checking out Revision 10716bfb800e562c74945a34acaa4cbddaad1552 (detached)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 10716bfb800e562c74945a34acaa4cbddaad1552 # timeout=10
Commit message: "set ignore masking to True by default"
 > git rev-list --no-walk 7e7e5e3e90d0c972e80303dc8f4422e132ac6b2a # timeout=10
[transformers4rec_tests] $ /bin/bash /tmp/jenkins13522023760786896743.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.3, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/transformers4rec_tests/transformers4rec
plugins: anyio-3.6.1, xdist-2.5.0, forked-1.4.0, cov-4.0.0
collected 1 item

tests/unit/test_notebooks.py . [100%]

============================== 1 passed in 33.80s ==============================
Performing Post build task...
Match found for : : True
Logical operation result is TRUE
Running script : #!/bin/bash
cd /var/jenkins_home/
CUDA_VISIBLE_DEVICES=2 python test_res_push.py "https://api.GitHub.com/repos/NVIDIA-Merlin/Transformers4Rec/issues/$ghprbPullId/comments" "/var/jenkins_home/jobs/$JOB_NAME/builds/$BUILD_NUMBER/log"
[transformers4rec_tests] $ /bin/bash /tmp/jenkins9779616723547339963.sh

@github-actions
Copy link

github-actions bot commented Oct 5, 2022

tests/torch/test_losses.py Show resolved Hide resolved
transformers4rec/torch/features/sequence.py Show resolved Hide resolved
@@ -274,7 +274,7 @@ def calculate_metrics( # type: ignore

outputs = {}
if forward:
predictions = self(predictions)
predictions = self(predictions, ignore_masking=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

why False?

Copy link
Contributor Author

@sararb sararb Oct 6, 2022

Choose a reason for hiding this comment

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

To compute metrics and losses we need to compute the masked targets. So we should not ignore masking

@nvidia-merlin-bot
Copy link

Click to view CI Results
GitHub pull request #498 of commit 6be97b9272bd97ff4576309aa517725f7343340d, no merge conflicts.
Running as SYSTEM
Setting status of 6be97b9272bd97ff4576309aa517725f7343340d to PENDING with url http://10.20.17.181:8080/job/transformers4rec_tests/225/ and message: 'Build started for merge commit.'
Using context: Jenkins Unit Test Run
Building on master in workspace /var/jenkins_home/workspace/transformers4rec_tests
using credential nvidia-merlin-bot
Cloning the remote Git repository
Cloning repository https://github.com/NVIDIA-Merlin/Transformers4Rec.git
 > git init /var/jenkins_home/workspace/transformers4rec_tests/transformers4rec # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/Transformers4Rec.git
 > git --version # timeout=10
using GIT_ASKPASS to set credentials This is the bot credentials for our CI/CD
 > git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/Transformers4Rec.git +refs/heads/*:refs/remotes/origin/* # timeout=10
 > git config remote.origin.url https://github.com/NVIDIA-Merlin/Transformers4Rec.git # timeout=10
 > git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # timeout=10
 > git config remote.origin.url https://github.com/NVIDIA-Merlin/Transformers4Rec.git # timeout=10
Fetching upstream changes from https://github.com/NVIDIA-Merlin/Transformers4Rec.git
using GIT_ASKPASS to set credentials This is the bot credentials for our CI/CD
 > git fetch --tags --force --progress -- https://github.com/NVIDIA-Merlin/Transformers4Rec.git +refs/pull/498/*:refs/remotes/origin/pr/498/* # timeout=10
 > git rev-parse 6be97b9272bd97ff4576309aa517725f7343340d^{commit} # timeout=10
Checking out Revision 6be97b9272bd97ff4576309aa517725f7343340d (detached)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 6be97b9272bd97ff4576309aa517725f7343340d # timeout=10
Commit message: "Merge branch 'main' into ignore-masking"
 > git rev-list --no-walk 0f5b64bf82ddf0857da7b74f504fa92238d6112f # timeout=10
[transformers4rec_tests] $ /bin/bash /tmp/jenkins16112933851064363462.sh
============================= test session starts ==============================
platform linux -- Python 3.8.10, pytest-7.1.3, pluggy-1.0.0
rootdir: /var/jenkins_home/workspace/transformers4rec_tests/transformers4rec
plugins: anyio-3.6.1, xdist-2.5.0, forked-1.4.0, cov-4.0.0
collected 1 item

tests/unit/test_notebooks.py . [100%]

============================== 1 passed in 34.83s ==============================
Performing Post build task...
Match found for : : True
Logical operation result is TRUE
Running script : #!/bin/bash
cd /var/jenkins_home/
CUDA_VISIBLE_DEVICES=2 python test_res_push.py "https://api.GitHub.com/repos/NVIDIA-Merlin/Transformers4Rec/issues/$ghprbPullId/comments" "/var/jenkins_home/jobs/$JOB_NAME/builds/$BUILD_NUMBER/log"
[transformers4rec_tests] $ /bin/bash /tmp/jenkins7998668108975607325.sh

@karlhigley
Copy link
Contributor

Looks good from a serving standpoint 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/inference enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task] Update model_pt.py to incorporate ignore_masking arg in TF4Rec model serving with Triton
4 participants