-
Notifications
You must be signed in to change notification settings - Fork 27.5k
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
Generate: skip left-padding tests on old models #23437
Conversation
@@ -1516,3 +1516,7 @@ def test_retain_grad_hidden_states_attentions(self): | |||
|
|||
def test_save_load_fast_init_from_base(self): | |||
pass | |||
|
|||
@unittest.skip("The model doesn't support left padding") # and it's not used enough to be worth fixing :) |
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.
Note that this is not Bart, but the BartDecoder used as a stand-alone model. 11 out of the 16 skips below follow this pattern, a decoder used as a stand-alone model (which is not very used in practice, afaik).
The exceptions are CTRL, ImageGPT, Reformer, and TransfoXL
The documentation is not available anymore as the PR was closed or merged. |
It seems like |
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.
Thank you! I am more than happy to see a cleaner daily CI report + You are the king of generation --> 200% approval for this PR from my side.
@gante it all passes now 😄 |
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.
Thanks for adding!
What does this PR do?
We have a few left-padding tests polluting our daily CI -- these are from old models, where it is not worth committing >1hr per model to add support for left padding.
This PR skips the test in those models, so we can focus our energy where it matters :)