Skip to content

Refactor & enable JIT tests in all models and add warnings if skipped #3033

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

Merged
merged 4 commits into from
Nov 20, 2020

Conversation

datumbox
Copy link
Contributor

@datumbox datumbox commented Nov 19, 2020

Fixes #3028

Initial commits of this PR should have caused the following models to fail:

  • googlenet
  • inceptionv3
  • maskrcnn

The failures were related to non JIT problems on the tests but we could not see them because the tests were not running.

This PR fixes the problems on the above tests, refactors the unit-tests and re-enables JIT tests for all models.

@datumbox
Copy link
Contributor Author

@fmassa Unfortunately the tests did not fail which means that the JIT scripts did not run. This can be confirm by checking any of the run logs for the following warnings:

test/test_models.py::ModelTester::test_alexnet_cpu
  /Users/distiller/project/test/common_utils.py:281: RuntimeWarning: The checkModule test for AlexNet was skipped. This test checks if the module's results in TorchScript match eager and that it can be exported. To run these tests make sure you set the environment variable PYTORCH_TEST_WITH_SLOW=1 and that the test is not manually skipped.
    warnings.warn(msg, RuntimeWarning)

Any ideas on how we can turn on the PYTORCH_TEST_WITH_SLOW env var at CI? Alternatively we can hardcode it here until this is fixed:

TEST_WITH_SLOW = os.getenv('PYTORCH_TEST_WITH_SLOW', '0') == '1'
# TEST_WITH_SLOW = True # TODO: Delete this line once there is a PYTORCH_TEST_WITH_SLOW aware CI job

@fmassa
Copy link
Member

fmassa commented Nov 20, 2020

Thanks a lot for looking into this, we should get this fixed quickly.

I think we should try to switch the flag to always run the slow tests, and we should see if CI timeout or not.
If you want to set the env var for tests in CI only, you can add it in

python -m torch.utils.collect_env
and
python -m torch.utils.collect_env

It's likely to fail for models:
maskrcnn

Just note that we have a separate test checking detection models that they run on torchscript, so we should be safe there, see

vision/test/test_models.py

Lines 213 to 215 in a884cb7

scripted_model = torch.jit.script(model)
scripted_model.eval()
scripted_out = scripted_model(model_input)[1]

@datumbox datumbox changed the title [WIP] Enable JIT tests in all models and add warnings if they are skipped Enable JIT tests in all models and add warnings if they are skipped Nov 20, 2020
@datumbox datumbox changed the title Enable JIT tests in all models and add warnings if they are skipped Refactor & enable JIT tests in all models and add warnings if skipped Nov 20, 2020
@datumbox datumbox requested a review from fmassa November 20, 2020 11:38
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@fmassa fmassa merged commit 4521f6d into pytorch:master Nov 20, 2020
@datumbox datumbox deleted the bugfix/all_models_jit branch November 20, 2020 16:12
bryant1410 pushed a commit to bryant1410/vision-1 that referenced this pull request Nov 22, 2020
…pytorch#3033)

* Enable jit tests in all models and add warning if checkModule() tests are skipped.

* Turning on JIT tests on CI.

* Fixing broken unit-tests.

* Refactoring and cleaning up duplicate code.
vfdev-5 pushed a commit to Quansight/vision that referenced this pull request Dec 4, 2020
…pytorch#3033)

* Enable jit tests in all models and add warning if checkModule() tests are skipped.

* Turning on JIT tests on CI.

* Fixing broken unit-tests.

* Refactoring and cleaning up duplicate code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

torchscript model tests have blind spots
3 participants