-
Notifications
You must be signed in to change notification settings - Fork 34.1k
Remove many output_attentions and other traced outputs on 100+ models #43590
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
Changes from all commits
e253ef2
4183863
49b1547
9382257
e6c4b89
6c002b5
0ca6f96
b327b19
807b17f
f0916ea
24bf3b5
15bf2d7
a0569c8
8ae95c5
389dde9
21b65ef
37e2841
6affc73
b5f3996
2b53f63
7867d98
6c22616
becb675
41257e2
7961fba
2147358
531919c
77e4af8
67dc0e9
79cdb8c
7b62ce5
bf77c9e
6d24a2b
edb124e
fe0df51
cb2d2c8
4f4b25a
4c51e57
3002e8a
ba87a9f
6137c6b
96f4f73
ee96236
7f81eb1
21ec95b
492d5a2
764541b
84b1322
6864ff2
082de92
059afac
a4d3535
e53623f
73aac50
4f00326
a34d300
dbbc8e9
9376382
e79d92a
aeb63ff
2009016
aac3dc8
9203684
3d96885
c6415a2
15f8fe2
81aafcf
af2dc85
1827905
45fedbd
6cd2ef6
e1aa015
5fca01d
43ffe22
e452559
2add677
6ed2c4e
4fc5abf
f8a35e5
93a1838
303140e
20e04ed
8adbf03
4731344
dc95ece
566e1d2
f8e995c
6bf065b
b9ed56c
235875d
6c01379
f71cd4c
93c1c5d
73e92ca
fb86e68
0fde2ff
a4043d9
9c9f942
572d4b0
9890f73
c693ee8
68cbdb7
5b0e219
f382c16
7ba0b70
5dacd51
748eb9f
511ba7e
2fc22d5
a2d92d2
22b2e28
e5620be
e8f2f6c
0c1d62b
f8e9001
780ed8e
b96e5ca
ca46369
9b00048
032f339
3021ea8
5b47a27
d7c7b69
4ce0480
eb064c4
555c570
5c931db
27a08f5
63f0f0c
33d2482
f0ef4d1
d138b47
4f5b1de
c9d4b1a
7c6b0e0
27f539d
d26056f
28038bd
206f2b1
1d7046a
fb19136
8112bc5
96dfc6a
5cb7245
90e392d
b9b5974
af47d89
c08dc4e
4be2830
2c87457
eb0a5f7
82dd509
314415e
7b97114
5e963f8
1c8dc74
0c1bf96
a051182
c0fe07f
7280031
19d20c2
aecbf09
d719a26
074579f
d5a88ef
60f7b1f
e95ff50
655bc47
3e40757
433f817
59b18b1
ce1c616
f0cc979
b034aa5
9b7cef9
12298cd
b9890c8
3887673
10d278e
14029fe
c1b69ce
346cc0f
a8b8fd4
fdaa667
e8b24d1
2e30c62
b1e7d9f
2c1678f
d6fb85a
0a647b5
6770bea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -765,7 +765,7 @@ def _prepare_encoder_decoder_kwargs_for_generation( | |
| add_hook_to_module(encoder, AlignDevicesHook(io_same_device=True)) | ||
|
|
||
| # 2. Prepare encoder args and encoder kwargs from model kwargs and generation config. | ||
| irrelevant_prefix = ["decoder_", "cross_attn", "use_cache"] | ||
| irrelevant_prefix = ["decoder_", "cross_attn", "use_cache", "past_key_values", "cache_params"] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is needed as encoder oftentimes share the same attention module from the decoder, meaning that if we pass the cache there everything gets messy
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Humm, I don't get why we need this suddenly? Even encoder may want the cache with EncoderDecoder cache no?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the encoder module itself never wants a cache, it's only forwarded once and then "saved" for each subsequent step. Only the decoder needs the cache to properly overwrite states. If the encoder also gets the cache, then it can update the cache as well which makes generate being broken for certain methods (not sure which anymore but CI was broken for a few tests on bart then).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ha yes, cause you made sure to propagate kwargs now as well! Makes sense then! |
||
| encoder_kwargs = { | ||
| argument: value | ||
| for argument, value in model_kwargs.items() | ||
|
|
||
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.
This is the new wrapper to make backbones behave like expected: They always output hidden states so we control this here.
output = type(output)(**filtered_output_data)is a bit weird but it allows us to construct our modeling outputs properly as there is no delete function and I don't think we want one