-
Notifications
You must be signed in to change notification settings - Fork 27.3k
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
remvoe _create_and_check_torch_fx_tracing
in specific test files
#18667
remvoe _create_and_check_torch_fx_tracing
in specific test files
#18667
Conversation
The documentation is not available anymore as the PR was closed or merged. |
_create_and_check_torch_fx_tracing
in specific test files
The common case test for TorchScript, and if I recall correctly there was an issue for those models on that aspect? You suggest to add a flag called Also, I think that some of those models can actually be torchscripted with torch 1.12, but the issue was that we are (were?) testing in torch 1.11. |
There might be before. But as far as I can tell, the issue probably came from the input and label names preparation. As the tests pass after I remove their re-definitions from the specific model test files, I think it's fine and better to clean them up. (The only failure is from Hubert).
This is to allow torch trace test still run while skip the torch script test, as currently Hubert test will fail on torchscript.
We can re-evaluate this, but again, let's not to do changes regarding this part in this PR. This PR is merely to avoid overwriting |
f9852e1
to
72a0d60
Compare
72a0d60
to
32437fd
Compare
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 cleaning this up. Re-new flag, I am still opposed for the same reasons as in the comment linked above. Everything should be controlled by one fx_compatible
flag, we don't want multiple ones :-)
@michaelbenayoun If this PR is OK on your side, I am going to merge. Regarding the flag, let's see what we can do in a separate PR. |
32437fd
to
e1186a8
Compare
…uggingface#18667) * remvoe _create_and_check_torch_fx_tracing defined in specific model test files Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
What does this PR do?
Remvoe
_create_and_check_torch_fx_tracing
in specific model test files, as the common one can handle them correctly.The only exception is
Hubert
model, but we can also remove it, and setfx_compatible
toFalse
(just as forWav2Vec2
).It might be better to add
torch_script_compatible
to handleHubert
and related models.Motivation: Make the change in #18547 available to all tests.