Add is_causal API for TransformerDecoder
#97166
Conversation
The same API is implemented for `TransformerEncoder`, where this argument is passed through to the sublayers.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/97166
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 8808120: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
cpuhrsch
added
the
triaged
|
@mikekgfb has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
tgt → memory Previous error was replicated due to copy-pasting; now fixed for both occurrences.
|
@janEbert should this also cover |
|
Sorry, I think my message was unclear. I meant that the API is already implemented for |
|
There’s a top level transformer module as well. My question referred to that module. (`class Transformer(Module):` starting at L17 of trnasformer.py)
(This can be a separate PR if you want to submit that.)
|
|
@mikekgfb has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@pytorchbot merge -f "updated diff" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
@janEbert can you please submit a PR that updates the definition of *_is_causal in docstrings, based on the change in #97214 which replaces the mutual exclusivity and makes *_is_causal a hint. (Bad things happen when the hint incorrectly asserts the attention mask is causal, when it is not. The other direction works fine when is_causal is False but the matrix actually contains a causal mask) |
|
@mikekgfb Thanks for merging already. :) |
|
👍
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: janEbert ***@***.***>
Sent: Friday, March 24, 2023 11:28:33 PM
To: pytorch/pytorch ***@***.***>
Cc: Michael Gschwind ***@***.***>; Mention ***@***.***>
Subject: Re: [pytorch/pytorch] Add `is_causal` API for `TransformerDecoder` (PR #97166)
@mikekgfb Thanks for merging already. :) I'll gladly do both things you mentioned, updating the docstring and implementing the API for Transformer as well. Cheers! — Reply to this email directly, view it on GitHub, or unsubscribe. You
ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
ZjQcmQRYFpfptBannerEnd
@mikekgfb<https://github.com/mikekgfb> Thanks for merging already. :)
I'll gladly do both things you mentioned, updating the docstring and implementing the API for Transformer as well. Cheers!
—
Reply to this email directly, view it on GitHub<#97166 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AOT4XHIWHSQQ46SYHWYWFH3W5YNZDANCNFSM6AAAAAAWBOPDRU>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
According to pytorch#97214, as discussed in pytorch#97166.
As discussed in pytorch#97166.
The same API is implemented for
TransformerEncoder, where this argument is passed through to the sublayers.