Skip to content

Set ignore_masking to True by default#498

Merged
rnyak merged 2 commits intomainfrom
ignore-masking
Oct 10, 2022
Merged

Set ignore_masking to True by default#498
rnyak merged 2 commits intomainfrom
ignore-masking

Conversation

@sararb
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown

github-actions bot commented Oct 5, 2022

Comment thread tests/torch/test_losses.py
Comment thread transformers4rec/torch/features/sequence.py
outputs = {}
if forward:
predictions = self(predictions)
predictions = self(predictions, ignore_masking=False)
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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