Skip to content

Conversation

@gs-olive
Copy link
Contributor

@gs-olive gs-olive commented Mar 10, 2023

Description

  • Add testing for Torch 1.13.1 path in CI across both TS and FX compilation paths
  • Disable aten tests for 1.13, to resolve Torch Dynamo import/functionality issues
  • Add parameter fields to CI to accomodate Torch 1.13.1 version
  • Update dispatch_tracer function docstrings and imports to avoid naming issue with torch._dynamo vs torchdynamo

See this CI run for a sample of the 1.13.1 run results. Currently the only failing tests are test_conv_add_standalone_module from the FX quantization path, and these failures may arise from API usage changes between 1.13 and 2.0.

The CI is now passing with the updates to the FX quantization path. See this CI run.

The following tests are disabled for the 1.13.1 CI workflow, due to Dynamo compatibility issues:

  • tracer/test_dispatch_tracer.py
  • tracer/test_resnet.py
  • tracer/test_resnet.py
  • converters/aten_op/*

Fixes #1716

Type of change

Please delete options that are not relevant and/or add your own.

  • Infrastructure/CI Change

Checklist:

  • [ x ] My code follows the style guidelines of this project (You can use the linters)
  • [ x ] I have performed a self-review of my own code
  • [ x ] I have commented my code, particularly in hard-to-understand areas and hacks
  • [ x ] I have made corresponding changes to the documentation
  • [ x ] I have added tests to verify my fix or my feature
  • [ x ] New and existing unit tests pass locally with my changes
  • [ x ] I have added the relevant labels to my PR in so that relevant reviewers are notified

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

tracing_mode: str = "real",
dynamo_config: Optional[DynamoConfig] = None,
) -> Tuple[torch.fx.GraphModule, Set[Guard]]:
) -> Tuple[torch.fx.GraphModule, Set]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Guard specification here was removed since this file still needs to be imported even when Torch 1.13 is used (torch_tensorrt.dump_build_info() automatically imports this). We cannot import the Guard module with Torch 1.13 since the Dynamo import is disabled on 1.13.

@gs-olive gs-olive added the WIP Work is in progress, pull request should not be merged yet label Mar 10, 2023
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

- Add testing for Torch 1.13.1 path in CI across both TS and FX
compilation paths
- Disable `aten` tests for 1.13.1, to resolve Torch Dynamo
import/functionality issues
- Refactor quantization FX tests to resolve key error in pattern
dictionary
- Add parameter fields to CI to accomodate Torch 1.13.1 version
- Update `dispatch_tracer` function docstrings and imports to avoid
naming issue with `torch._dynamo` vs `torchdynamo`
- Rename CI versioning to use "legacy"
@gs-olive gs-olive force-pushed the pytorch_1.13_ci_testing branch from 66cc20f to 7fa598c Compare March 11, 2023 02:48
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

@gs-olive gs-olive requested a review from narendasan March 11, 2023 02:50
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to C++ style guidelines

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code conforms to Python style guidelines

@gs-olive gs-olive removed the WIP Work is in progress, pull request should not be merged yet label Mar 11, 2023
Copy link
Collaborator

@narendasan narendasan left a comment

Choose a reason for hiding this comment

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

LGTM

@narendasan narendasan merged commit 72f4bce into main Mar 13, 2023
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.

Add back 1.13 testing to CI for legacy support

4 participants