-
Notifications
You must be signed in to change notification settings - Fork 25.6k
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
Fix FlauBERT GPU test #6142
Fix FlauBERT GPU test #6142
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6142 +/- ##
==========================================
- Coverage 78.35% 78.32% -0.04%
==========================================
Files 146 146
Lines 26403 26403
==========================================
- Hits 20689 20679 -10
- Misses 5714 5724 +10
Continue to review full report at Codecov.
|
if lengths is None: | ||
if input_ids is not None: | ||
lengths = (input_ids != self.pad_index).sum(dim=1).long() | ||
else: | ||
lengths = torch.LongTensor([slen] * bs) | ||
lengths = torch.tensor([slen] * bs, device=device) |
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.
Why not LongTensor?
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.
LongTensor isn't a factory to create tensors, but the tensor class, so you can't directly set the device type on it. You would have to do:
lengths = torch.LongTensor([slen] * bs).to(device)
If I understand correctly, this first creates the tensor on the CPU, before creating a new one on the updated device (if the device is different). When using the factory method, I believe it's only created once, and is therefore more efficient. I didn't check the source code directly though so I may be wrong.
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.
Makes sense, thanks for clarifying!
No description provided.