Part of #109684
- #109684
Changes:
- Add new functions `tree_structure`, `tree_leaves`, `tree_map_` and `tree_map_only_` to Python pytree.
- Extract reusable tests for pytree to `TestGenericPytree`.
- Change `treespec_dumps` and `treespec_loads` in C++ pytree to call Python pytree and use JSON string as serialization type.
- Rename `torch.utils.pytree` -> `torch.utils._cxx_pytree`.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/110395
Approved by: https://github.com/zou3519
This added the numpy typing plugin to mypy config so that we could
use it for DeviceMesh typing annotations
Please see https://github.com/pytorch/pytorch/pull/92931 about why we need this. For example, we are currently saving the DeviceMesh's mesh field as torch.Tensor, where when we do sth like:
```python
with FakeTensorMode():
device_mesh = DeviceMesh("cuda", torch.arange(4))
```
It would throw error because FakeTensorMode or any TorchDispatchMode tracks every tensor creation and interactions. While DeviceMesh just want to save a nd-array to record the mesh topology, and would like to avoid the interaction with subsystems like FakeTensor, so we want to support saving `mesh` as numpy array instead.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/92930
Approved by: https://github.com/ezyang, https://github.com/malfet
Summary:
Changes including:
- introduced `linter/`, `testing/`, `stats/` folders in `tools/`
- move appropriate scripts into these folders
- change grepped references in the pytorch/pytorch repo
Next step
- introduce `build/` folder for build scripts
Pull Request resolved: https://github.com/pytorch/pytorch/pull/60473
Test Plan:
- CI (this is important b/c pytorch/test-infra also rely on some script reference.
- tools/tests/
Reviewed By: albanD
Differential Revision: D29352716
Pulled By: walterddr
fbshipit-source-id: bad40b5ce130b35dfd9e59b8af34f9025f3285fd
Summary:
During development it is common practice to put `type: ignore` comments on lines that are correct, but `mypy` doesn't recognize this. This often stems from the fact, that the used `mypy` version wasn't able to handle the used pattern.
With every new release `mypy` gets better at handling complex code. In addition to fix all the previously accepted but now failing patterns, we should also revisit all `type: ignore` comments to see if they are still needed or not. Fortunately, we don't need to do it manually: by adding `warn_unused_ignores = True` to the configuration, `mypy` will error out in case it encounters an `type: ignore` that is no longer needed.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/60006
Reviewed By: jbschlosser, malfet
Differential Revision: D29133237
Pulled By: albanD
fbshipit-source-id: 41e82edc5cd5affa7ccedad044b59b94dad4425a
Summary:
This PR greatly simplifies `mypy-strict.ini` by strictly typing everything in `.github` and `tools`, rather than picking and choosing only specific files in those two dirs. It also removes `warn_unused_ignores` from `mypy-strict.ini`, for reasons described in https://github.com/pytorch/pytorch/pull/56402#issuecomment-822743795: basically, that setting makes life more difficult depending on what libraries you have installed locally vs in CI (e.g. `ruamel`).
Pull Request resolved: https://github.com/pytorch/pytorch/pull/59117
Test Plan:
```
flake8
mypy --config mypy-strict.ini
```
Reviewed By: malfet
Differential Revision: D28765386
Pulled By: samestep
fbshipit-source-id: 3e744e301c7a464f8a2a2428fcdbad534e231f2e
Summary:
This PR simplifies `.github/scripts/generate_ci_workflows.py` by using the same strategy as https://github.com/pytorch/pytorch/issues/54344, representing workflows as plain data to avoid duplicating the definition of the `generate_workflow_file` function. This will make the script easier to maintain if/when that function is modified and/or more workflow types are added.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/58491
Test Plan:
The Lint job in CI; specifically:
```
make generate-gha-workflows
mypy --config mypy-strict.ini
```
Reviewed By: malfet, seemethere
Differential Revision: D28511918
Pulled By: samestep
fbshipit-source-id: aaf415a954d938a29aee7c9367c9bc2b9f44bb01
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/58001
Adds a script so that devs can generate a commit (at the base of a stack) that removes all CI jobs but the set that they care about. See CONTRIBUTING.md changes for usage
Test Plan: Imported from OSS
Reviewed By: mruberry
Differential Revision: D28359910
Pulled By: driazati
fbshipit-source-id: 2741570f2bab2c28f4a9d7aef727b1b2399d0ce1
Summary:
Judging from https://github.com/pytorch/pytorch/issues/57584, it seems like the test-reports artifact was originally intended to be downloaded to `$PWD/test-reports` instead of just directly into `$PWD`. To minimize confusion, this PR changes it to download into `test/test-reports`, which should match where the files came from in the `test` step anyway.
TODOs:
- [x] change the extract path for test-reports
- [x] install Python dependencies
- [x] call `tools/print_test_stats.py`
- [x] use deep clone to allow `git` commands
- [x] correctly set `CIRCLE_*` environment variables
- [x] set Scribe credentials
- [x] set AWS credentials
Pull Request resolved: https://github.com/pytorch/pytorch/pull/57647
Test Plan: CI.
Reviewed By: seemethere
Differential Revision: D28325833
Pulled By: samestep
fbshipit-source-id: cc322bad76747f59b764a1a0a863153bb26095e7
Summary:
This is a second attempt at https://github.com/pytorch/pytorch/issues/51214. It should achieve the same goals with (as far as I can tell) no disadvantages, but the advantages are a bit less pronounced than in the more dictatorial approach that https://github.com/pytorch/pytorch/issues/51214 took:
- Unfortunately, I was unable to figure out how to include [the `mypy` configuration given in the docstring of `tools.mypy_wrapper.main`](7115a4b870/tools/mypy_wrapper.py (L81-L89)), because as walterddr pointed out, `"${env:HOME}/miniconda3/envs/pytorch/bin/python"` is not guaranteed to be correct on everyone's machine:
```json
{
"python.linting.enabled": true,
"python.linting.mypyEnabled": true,
"python.linting.mypyPath": "${env:HOME}/miniconda3/envs/pytorch/bin/python",
"python.linting.mypyArgs": [
"${workspaceFolder}/tools/mypy_wrapper.py"
]
}
```
Importantly, this does not work:
```json
"python.linting.mypyPath": "${workspaceFolder}/tools/mypy_wrapper.py"
```
This is because VS Code does not run the given `mypy` command inside of the user's specified virtual environment, so for instance, on my system, setting the `mypy` command to directly call `tools/mypy_wrapper.py` results in using `mypy 0.782` instead of the correct `mypy 0.812`.
Sadly, [this](https://code.visualstudio.com/docs/editor/variables-reference#_configuration-variables) does not work either, although I'm not sure why:
```json
{
"python.linting.mypyPath": "${config:python.pythonPath}",
"python.linting.mypyArgs": [
"${workspaceFolder}/tools/mypy_wrapper.py"
]
}
```
- As a result, `git clean -fdx; tools/vscode_settings.py` still results in some loss of useful configuration.
One other thing to note: as `.vscode/settings_recommended.json` shows, there are some configuration sections that only take effect within the context of a `"[language]"`, so currently, if a dev already has one of those settings, it would be entirely overwritten by `tools/vscode_settings.py` rather than a graceful merge. This could probably be fixed by using a deep merge instead of the current shallow merge strategy.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/57671
Test Plan:
If you want, you can typecheck the small script added by this PR (no output is expected):
```sh
tools/mypy_wrapper.py $PWD/tools/vscode_settings.py
```
You can also try running it to update your own VS Code workspace settings:
```sh
tools/vscode_settings.py
```
This should have minimal impact on your existing `tools/settings.json` file other than enabling the few explicitly recommended settings (e.g. it should not reorder or remove any of your existing settings).
Reviewed By: malfet
Differential Revision: D28230390
Pulled By: samestep
fbshipit-source-id: 53a7907229e5807c77531cae4f9ab9d469fd7684
Summary:
See https://github.com/pytorch/pytorch/pull/56523#issuecomment-823562134 for context. Basically the idea is that people (including myself) keep assuming that the single-asterisk `*` wildcard means "match in this directory and in its subdirectories", which is _not_ true. Removing the wildcards thus reduces confusion.
Ideally I would like to remove _all_ of these wildcards and then add a lint to disallow them in the future (and also greatly simplify the pattern-matching logic in `tools/mypy_wrapper.py`; see https://github.com/pytorch/pytorch/issues/55702 for context), but currently this one can't be removed:
```
tools/autograd/*.py,
```
That is because there is a file called `tools/autograd/templates/annotated_fn_args.py` (added in https://github.com/pytorch/pytorch/issues/41575) which is not a valid Python file and thus cannot be checked by `mypy`. ezyang would it be possible to rename that file to use a suffix other than `.py`?
Pull Request resolved: https://github.com/pytorch/pytorch/pull/56645
Test Plan:
```
$ mypy
Success: no issues found in 1317 source files
$ mypy --config=mypy-strict.ini
Success: no issues found in 72 source files
```
The numbers of source files should be the same before and after this PR.
Reviewed By: ezyang
Differential Revision: D27925207
Pulled By: samestep
fbshipit-source-id: c17faf73665a75393d3109346a1138c2af023abb
Summary:
This queries the local git repo for changed files (any changed files, not just committed ones) and sends them to mypy/flake8 instead of the default (which is the whole repo, defined the .flake8 and mypy.ini files). This brings a good speedup (from 15 seconds with no cache to < 1 second from my local testing on this PR).
```bash
make quicklint -j 6
```
It should be noted that the results of this aren’t exactly what’s in the CI, since mypy and flake8 ignore the `include` and `exclude` parts of their config when an explicit list of files is passed in.
](https://our.intern.facebook.com/intern/diff/27901577/)
Pull Request resolved: https://github.com/pytorch/pytorch/pull/56559
Pulled By: driazati
Reviewed By: malfet
Differential Revision: D27901577
fbshipit-source-id: 99f351cdfe5aba007948aea2b8a78f683c5d8583
Summary:
This should make it easier to resolve issues surfaced by https://github.com/pytorch/pytorch/issues/56290. Also see https://github.com/pytorch/pytorch/pull/56559#discussion_r617828152 for context.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/56616
Test Plan:
You could add a type error in a strict-checked file like `tools/test_history.py`, and then run this command:
```
$ mypy --config=mypy-strict.ini tools/test_history.py
```
Output before this PR:
```
tools/test_history.py:13:1: error: Function is missing a type annotation for one or more arguments
Found 1 error in 1 file (checked 1 source file)
```
Output after this PR:
```
tools/test_history.py:13:1: error: Function is missing a type annotation for one or more arguments [no-untyped-def]
Found 1 error in 1 file (checked 1 source file)
```
Reviewed By: driazati
Differential Revision: D27918753
Pulled By: samestep
fbshipit-source-id: 953926e019a7669da9004fd54498b414aec777a6
Summary:
Resolves https://github.com/pytorch/pytorch/issues/55810 by closing some possible security holes due to using [GitHub Actions `${{ <expressions> }}`](https://docs.github.com/en/actions/reference/context-and-expression-syntax-for-github-actions#about-contexts-and-expressions) in `.github/workflows/add_annotations.yml` and also patching a few other possible scenarios that could cause the workflow to fail by a PR passing a malformed artifact.
- [x] flag and remove GitHub Actions expressions in JS scripts
- [x] don't fail the workflow if the artifact doesn't look as expected
- [x] write unit tests for `tools/extract_scripts.py`
Pull Request resolved: https://github.com/pytorch/pytorch/pull/56071
Test Plan:
I tested the end-to-end "Lint" and "Add annotations" system in a separate sandbox repo, including the following cases:
- well-formed artifact
- missing artifact
- artifact containing a file named `linter-output.zip` (name clash)
- artifact whose `commit-sha.txt` doesn't contain a 40-digit hex string
- artifact whose `commit-sha.txt` contains a 40-digit hex string that isn't a valid Git hash for the current repo
- in this last case, the workflow does fail, but handling that is the responsibility of [pytorch/add-annotations-github-action](https://github.com/pytorch/add-annotations-github-action), not pytorch/pytorch
To run the new unit tests added in this PR:
```
python tools/test/test_extract_scripts.py
```
Reviewed By: seemethere
Differential Revision: D27807074
Pulled By: samestep
fbshipit-source-id: e2d3cc5437fe80ff03d46237ebba289901bc567c
Summary:
I noticed that https://github.com/pytorch/pytorch/issues/53296 added these two lines to the `files` list in `mypy-strict.ini`:
```
benchmarks/instruction_counts/*.py,
benchmarks/instruction_counts/*/*.py,
```
I opened https://github.com/pytorch/pytorch/issues/55700 to simplify them into one line, but I was also curious whether `tools/mypy_wrapper.py` correctly handles those patterns, so I added the `test_glob_wildcards_dont_expand_or_collapse` case shown in this PR. Turns out, it doesn't!
I believe this is because [`mypy` uses `glob`](https://github.com/python/mypy/blob/v0.770/mypy/config_parser.py#L45-L63) to parse these patterns, and for some reason, [`fnmatch`](https://docs.python.org/3/library/fnmatch.html) and [`glob`](https://docs.python.org/3/library/glob.html) don't agree with each other on what `*` means:
- according to `fnmatch`, `*` seems to mean `.*`
- according to `glob`, `*` seems to mean `[^/]*`
[This SO answer](https://stackoverflow.com/a/60174071) suggests using the [`glob.globmatch` function from the `wcmatch` library](https://facelessuser.github.io/wcmatch/glob/#globmatch) to solve the issue, but [we didn't want to add another external dependency](https://github.com/pytorch/pytorch/pull/55702#discussion_r610868623), so instead I simply modified our matching function to just directly call `mypy`'s own internal function that does the globbing (linked above).
One possible downside of this approach is that now the tests in `tools/test/test_mypy_wrapper.py` could break if the directory structure of PyTorch is changed.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/55702
Test Plan:
```
python tools/test/test_mypy_wrapper.py
```
Reviewed By: malfet, seemethere
Differential Revision: D27684499
Pulled By: samestep
fbshipit-source-id: d99387a579c21eee73d1714e3e815ab7155f9646
Summary:
These two lines were added in https://github.com/pytorch/pytorch/issues/53296, but they are needlessly complicated; this PR consolidates them.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/55700
Test Plan:
Run this command, and verify that the same number of files is given both before and after this PR:
```
mypy --config=mypy-strict.ini
```
Reviewed By: robieta
Differential Revision: D27684278
Pulled By: samestep
fbshipit-source-id: a34968cdff29cb8ad83813b277114224b5e37569
Summary:
This PR
- adds a `tools/translate_annotations.py` script that
- parses annotations into JSON using the regexes that we were previously passing to [`pytorch/add-annotations-github-action`](https://github.com/pytorch/add-annotations-github-action) and
- uses `git diff-index` to translate the line numbers for those annotations from the PR `merge` onto the PR `head`, since (as of https://github.com/pytorch/pytorch/issues/54967) we now run CI on the former instead of the latter;
- modifies the `flake8-py3` and `clang-tidy` jobs to use that script and thus upload JSON in their artifacts instead of raw text; and
- modifies the "Add annotations" workflow to specify `mode: json` to allow it to use those preprocessed annotations.
Depends on https://github.com/pytorch/add-annotations-github-action/pull/18.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/55569
Test Plan:
You can run the unit tests with this command:
```
python tools/test/test_translate_annotations.py
```
I also tested the entire system together in my personal sandbox repo.
Reviewed By: malfet
Differential Revision: D27662161
Pulled By: samestep
fbshipit-source-id: ecca51b79b9cf00c90fd89f0d41d0c7b89d69c63
Summary:
Add option to add //NOLINTNEXTLINE for every detected violation
Series of automated huge diffs are coming after this one to make large chunks of code clang-tidy
PR generated by new option: https://github.com/pytorch/pytorch/pull/55628
Pull Request resolved: https://github.com/pytorch/pytorch/pull/55612
Reviewed By: samestep
Differential Revision: D27649473
Pulled By: malfet
fbshipit-source-id: 251a68fcc50bf0fd69c6566293d4a516c0ab24c8
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/55227
This seems to increase the number of typechecked files.
Signed-off-by: Edward Z. Yang <ezyang@fb.com>
Test Plan: Imported from OSS
Reviewed By: janeyx99
Differential Revision: D27535373
Pulled By: ezyang
fbshipit-source-id: b36f6f8ce52c76848ed600ca9dd6b0c1de5813ff
Summary:
*Context:* https://github.com/pytorch/pytorch/issues/53406 added a lint for trailing whitespace at the ends of lines. However, in order to pass FB-internal lints, that PR also had to normalize the trailing newlines in four of the files it touched. This PR adds an OSS lint to normalize trailing newlines.
The changes to the following files (made in 54847d0adb9be71be4979cead3d9d4c02160e4cd) are the only manually-written parts of this PR:
- `.github/workflows/lint.yml`
- `mypy-strict.ini`
- `tools/README.md`
- `tools/test/test_trailing_newlines.py`
- `tools/trailing_newlines.py`
I would have liked to make this just a shell one-liner like the other three similar lints, but nothing I could find quite fit the bill. Specifically, all the answers I tried from the following Stack Overflow questions were far too slow (at least a minute and a half to run on this entire repository):
- [How to detect file ends in newline?](https://stackoverflow.com/q/38746)
- [How do I find files that do not end with a newline/linefeed?](https://stackoverflow.com/q/4631068)
- [How to list all files in the Git index without newline at end of file](https://stackoverflow.com/q/27624800)
- [Linux - check if there is an empty line at the end of a file [duplicate]](https://stackoverflow.com/q/34943632)
- [git ensure newline at end of each file](https://stackoverflow.com/q/57770972)
To avoid giving false positives during the few days after this PR is merged, we should probably only merge it after https://github.com/pytorch/pytorch/issues/54967.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/54737
Test Plan:
Running the shell script from the "Ensure correct trailing newlines" step in the `quick-checks` job of `.github/workflows/lint.yml` should print no output and exit in a fraction of a second with a status of 0. That was not the case prior to this PR, as shown by this failing GHA workflow run on an earlier draft of this PR:
- https://github.com/pytorch/pytorch/runs/2197446987?check_suite_focus=true
In contrast, this run (after correcting the trailing newlines in this PR) succeeded:
- https://github.com/pytorch/pytorch/pull/54737/checks?check_run_id=2197553241
To unit-test `tools/trailing_newlines.py` itself (this is run as part of our "Test tools" GitHub Actions workflow):
```
python tools/test/test_trailing_newlines.py
```
Reviewed By: malfet
Differential Revision: D27409736
Pulled By: samestep
fbshipit-source-id: 46f565227046b39f68349bbd5633105b2d2e9b19
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/53296
Part 1 of the instruction count microbenchmarks. This PR is focused on benchmark definition machinery. (Though you can run `main.py` to see it in action.) A summary of the system is given in the README.
Test Plan: Imported from OSS
Reviewed By: ngimel
Differential Revision: D26907092
Pulled By: robieta
fbshipit-source-id: 0f61457b3ce89aa59a06bf1f0e7a74ccdbf17090
Summary:
This PR
- moves `torch/testing/_internal/mypy_wrapper.py` (and its accompanying tests from `test/test_testing.py`) to `tools`,
- removes the now-unused `test_run_mypy` from `test/test_type_hints.py`, and
- replaces the hardcoded list of `mypy` configs (previously duplicated across `mypy_wrapper.py` and `.github/workflows/lint.yml`) with a simpler glob
Pull Request resolved: https://github.com/pytorch/pytorch/pull/54268
Test Plan:
Should also be run in the "Test tools" GHA workflow in CI:
```
python tools/test/test_mypy_wrapper.py
```
Reviewed By: janeyx99
Differential Revision: D27168095
Pulled By: samestep
fbshipit-source-id: a8dc18407b5e4c103ace23a636b0a8534951905a
Summary: Pull Request resolved: https://github.com/pytorch/pytorch/pull/54259
Test Plan:
The main point of this is to be run in our "Test tools" GitHub Actions workflow. To test locally:
```
mypy --config=mypy-strict.ini
python tools/test/test_test_history.py
```
Reviewed By: seemethere
Differential Revision: D27164519
Pulled By: samestep
fbshipit-source-id: 46f90e62e2d4d0c413b202419e509d471bad43de
Summary:
This is an initial attempt in refactoring and consolidating our S3 read logic for print_test_stats.py, test_history.py, and run_test.py. This way, boto3 and botocore do not need to be imported in various places throughout the code base, and duplicated logic (such as the many type definitions) can exist in one place: `tools/stat_utils/s3_stat_parser.py`. walterddr contributed to this PR by moving print_test_stats.py to the tools folder and the corresponding tests a subfolder within tools.
**NOTE: this removes those tests from CI as the new `tools/test/test_stats.py` is not in the test/ directory as the other tests in TESTS in run_test.py.**
Pull Request resolved: https://github.com/pytorch/pytorch/pull/53755
Test Plan:
This refactoring change should not break anything, so running the files as before should work as they did previously.
To make sure that print_test_stats.py still functions: run `python tools/test/test_stats.py` and make sure all tests pass.
To make sure that test_history.py works, run the example commands from `tools/test_history.py --help` and check that their output matches that shown. Note that the script will continue printing for a while, so don't be alarmed.
Some next steps:
- Actually coming up with similarities among the three current use cases and further refactoring/consolidating of functions (e.g., combining simplify and get_cases)
- Moving more parsing logic to s3_stat_parser.py to have better abstraction between our files
- Adding tests for s3_stat_parser.py when there is more functionality in it
Reviewed By: agolynski, samestep
Differential Revision: D27030285
Pulled By: janeyx99
fbshipit-source-id: e664781324ef7c0c30943bfd7f17c895075ef7a7
Summary:
This PR:
1. moves sharding algorithm from run_test.py to framework_utils.py (let me know if you have a better place for it)
2. adds tests for the algorithm in test_testing.py
3. fixes the algorithm so that it doesn't tack on the unknown jobs all to the shard with the minimum time, but instead distributes them around the shards.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/53942
Test Plan: python test/test_testing.py -k TestFrameworkUtils
Reviewed By: samestep
Differential Revision: D27047223
Pulled By: janeyx99
fbshipit-source-id: 824b20009c0bb707aa5361de445cdec795d5e3f1
Summary:
This PR adds a local [`mypy` plugin](https://mypy.readthedocs.io/en/stable/extending_mypy.html#extending-mypy-using-plugins) that warns if you accidentally run `mypy` using a version that doesn't match [the version we install for CI](6045663f39/.circleci/docker/common/install_conda.sh (L117)), since this trips people up sometimes when `mypy` gives errors in some versions (see https://github.com/pytorch/pytorch/issues/51513) but not others.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/51799
Test Plan:
To check that this doesn't break our `mypy` test(s) when you have the correct version installed:
```
python test/test_type_hints.py
```
To check that this does indeed warn when you have an incorrect `mypy` version installed, switch to a different version (e.g. 0.782), and run the above command or either of these:
```
mypy
mypy --config-file=mypy-strict.ini
```
You should get the following message on stderr:
```
You are using mypy version 0.782, which is not supported
in the PyTorch repo. Please switch to mypy version 0.770.
For example, if you installed mypy via pip, run this:
pip install mypy==0.770
Or if you installed mypy via conda, run this:
conda install -c conda-forge mypy=0.770
```
Reviewed By: janeyx99
Differential Revision: D26282010
Pulled By: samestep
fbshipit-source-id: 7b423020d0529700dea8972b27afa2d7068e1b12
Summary:
This is a followup to https://github.com/pytorch/pytorch/issues/49190. Vaguely speaking, the goals are to make it easy to identify test time regressions introduced by PRs. Eventually the hope is to use this information to edit Dr CI comments, but this particular PR just does the analysis and prints it to stdout, so a followup PR would be needed to edit the actual comments on GitHub.
**Important:** for uninteresting reasons, this PR moves the `print_test_stats.py` file.
- *Before:* `test/print_test_stats.py`
- *After:* `torch/testing/_internal/print_test_stats.py`
Notes on the approach:
- Just getting the mean and stdev for the total job time of the last _N_ commits isn't sufficient, because e.g. if `master` was broken 5 commits ago, then a lot of those job times will be much shorter, breaking the statistics.
- We use the commit history to make better estimates for the mean and stdev of individual test (and suite) times, but only when the test in that historical commit is present and its status matches that of the base commit.
- We list all the tests that were removed or added, or whose status changed (e.g. skipped to not skipped, or vice versa), along with time (estimate) info for that test case and its containing suite.
- We don't list tests whose time changed a lot if their status didn't change, because there's a lot of noise and it's unclear how to do that well without too many false positives.
- We show a human-readable commit graph that indicates exactly how many commits are in the pool of commits that could be causing regressions (e.g. if a PR has multiple commits in it, or if the base commit on `master` doesn't have a report in S3).
- We don't show an overall estimate of whether the PR increased or decreased the total test job time, because it's noisy and it's a bit tricky to aggregate stdevs up from individual tests to the whole job level. This might change in a followup PR.
- Instead, we simply show a summary at the bottom which says how many tests were removed/added/modified (where "modified" means that the status changed), and our best estimates of the mean times (and stdevs) of those changes.
- Importantly, the summary at the bottom is only for the test cases that were already shown in the more verbose diff report, and does not include any information about tests whose status didn't change but whose running time got much longer.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/50171
Test Plan:
To run the unit tests:
```
$ python test/test_testing.py
$ python test/print_test_stats.py
```
To verify that this works, check the [CircleCI logs](https://app.circleci.com/pipelines/github/pytorch/pytorch/258628/workflows/9cfadc34-e042-485e-b3b3-dc251f160307) for a test job run on this PR; for example:
- pytorch_linux_bionic_py3_6_clang9_test
To test locally, use the following steps.
First run an arbitrary test suite (you need to have some XML reports so that `test/print_test_stats.py` runs, but we'll be ignoring them here via the `--use-json` CLI option):
```
$ DATA_DIR=/tmp
$ ARBITRARY_TEST=testing
$ python test/test_$ARBITRARY_TEST.py --save-xml=$DATA_DIR/test/test_$ARBITRARY_TEST
```
Now choose a commit and a test job (it has to be on `master` since we're going to grab the test time data from S3, and [we only upload test times to S3 on the `master`, `nightly`, and `release` branches](https://github.com/pytorch/pytorch/pull/49645)):
```
$ export CIRCLE_SHA1=c39fb9771d89632c5c3a163d3c00af3bef1bd489
$ export CIRCLE_JOB=pytorch_linux_bionic_py3_6_clang9_test
```
Download the `*.json.bz2` file(s) for that commit/job pair:
```
$ aws s3 cp s3://ossci-metrics/test_time/$CIRCLE_SHA1/$CIRCLE_JOB/ $DATA_DIR/ossci-metrics/test_time/$CIRCLE_SHA1/$CIRCLE_JOB --recursive
```
And feed everything into `test/print_test_stats.py`:
```
$ bzip2 -kdc $DATA_DIR/ossci-metrics/test_time/$CIRCLE_SHA1/$CIRCLE_JOB/*Z.json.bz2 | torch/testing/_internal/print_test_stats.py --compare-with-s3 --use-json=/dev/stdin $DATA_DIR/test/test_$ARBITRARY_TEST
```
The first part of the output should be the same as before this PR; here is the new part, at the end of the output:
- https://pastebin.com/Jj1svhAn
Reviewed By: malfet, izdeby
Differential Revision: D26317769
Pulled By: samestep
fbshipit-source-id: 1ba06cec0fafac77f9e7341d57079543052d73db
Summary:
Closes https://github.com/pytorch/pytorch/issues/50513 by resolving all four checkboxes. If this PR is merged, I will also modify one or both of the following wiki pages to add instructions on how to use this `mypy` wrapper for VS Code editor integration:
- [Guide for adding type annotations to PyTorch](https://github.com/pytorch/pytorch/wiki/Guide-for-adding-type-annotations-to-PyTorch)
- [Lint as you type](https://github.com/pytorch/pytorch/wiki/Lint-as-you-type)
Pull Request resolved: https://github.com/pytorch/pytorch/pull/50826
Test Plan:
Unit tests for globbing function:
```
python test/test_testing.py TestMypyWrapper -v
```
Manual checks:
- Uninstall `mypy` and run `python test/test_type_hints.py` to verify that it still works when `mypy` is absent.
- Reinstall `mypy` and run `python test/test_type_hints.py` to verify that this didn't break the `TestTypeHints` suite.
- Run `python test/test_type_hints.py` again (should finish quickly) to verify that this didn't break `mypy` caching.
- Run `torch/testing/_internal/mypy_wrapper.py` on a few Python files in this repo to verify that it doesn't give any additional warnings when the `TestTypeHints` suite passes. Some examples (compare with the behavior of just running `mypy` on these files):
```sh
torch/testing/_internal/mypy_wrapper.py $PWD/README.md
torch/testing/_internal/mypy_wrapper.py $PWD/tools/fast_nvcc/fast_nvcc.py
torch/testing/_internal/mypy_wrapper.py $PWD/test/test_type_hints.py
torch/testing/_internal/mypy_wrapper.py $PWD/torch/random.py
torch/testing/_internal/mypy_wrapper.py $PWD/torch/testing/_internal/mypy_wrapper.py
```
- Remove type hints from `torch.testing._internal.mypy_wrapper` and verify that running `mypy_wrapper.py` on that file gives type errors.
- Remove the path to `mypy_wrapper.py` from the `files` setting in `mypy-strict.ini` and verify that running it again on itself no longer gives type errors.
- Add `test/test_type_hints.py` to the `files` setting in `mypy-strict.ini` and verify that running the `mypy` wrapper on it again now gives type errors.
- Change a return type in `torch/random.py` and verify that running the `mypy` wrapper on it again now gives type errors.
- Add the suggested JSON from the docstring of `torch.testing._internal.mypy_wrapper.main` to your `.vscode/settings.json` and verify that VS Code gives the same results (inline, while editing any Python file in the repo) as running the `mypy` wrapper on the command line, in all the above cases.
Reviewed By: walterddr
Differential Revision: D26049052
Pulled By: samestep
fbshipit-source-id: 0b35162fc78976452b5ea20d4ab63937b3c7695d
Summary:
Closes https://github.com/pytorch/pytorch/issues/50513 by resolving the first three checkboxes. If this PR is merged, I will also modify one or both of the following wiki pages to add instructions on how to use this `mypy` wrapper for VS Code editor integration:
- [Guide for adding type annotations to PyTorch](https://github.com/pytorch/pytorch/wiki/Guide-for-adding-type-annotations-to-PyTorch)
- [Lint as you type](https://github.com/pytorch/pytorch/wiki/Lint-as-you-type)
The test plan below is fairly manual, so let me know if I should add more automated tests to this PR.
Pull Request resolved: https://github.com/pytorch/pytorch/pull/50826
Test Plan:
Unit tests for globbing function:
```
python test/test_testing.py TestMypyWrapper -v
```
Manual checks:
- Uninstall `mypy` and run `python test/test_type_hints.py` to verify that it still works when `mypy` is absent.
- Reinstall `mypy` and run `python test/test_type_hints.py` to verify that this didn't break the `TestTypeHints` suite.
- Run `python test/test_type_hints.py` again (should finish quickly) to verify that this didn't break `mypy` caching.
- Run `torch/testing/_internal/mypy_wrapper.py` on a few Python files in this repo to verify that it doesn't give any additional warnings when the `TestTypeHints` suite passes. Some examples (compare with the behavior of just running `mypy` on these files):
```sh
torch/testing/_internal/mypy_wrapper.py README.md
torch/testing/_internal/mypy_wrapper.py tools/fast_nvcc/fast_nvcc.py
torch/testing/_internal/mypy_wrapper.py test/test_type_hints.py
torch/testing/_internal/mypy_wrapper.py torch/random.py
torch/testing/_internal/mypy_wrapper.py torch/testing/_internal/mypy_wrapper.py
```
- Remove type hints from `torch.testing._internal.mypy_wrapper` and verify that running `mypy_wrapper.py` on that file gives type errors.
- Remove the path to `mypy_wrapper.py` from the `files` setting in `mypy-strict.ini` and verify that running it again on itself no longer gives type errors.
- Add `test/test_type_hints.py` to the `files` setting in `mypy-strict.ini` and verify that running the `mypy` wrapper on it again now gives type errors.
- Remove type hints from `torch/random.py` and verify that running the `mypy` wrapper on it again now gives type errors.
- Add the suggested JSON from the docstring of `torch.testing._internal.mypy_wrapper.main` to your `.vscode/settings.json` and verify that VS Code gives the same results (inline, while editing any Python file in the repo) as running the `mypy` wrapper on the command line, in all the above cases.
Reviewed By: glaringlee, walterddr
Differential Revision: D25977352
Pulled By: samestep
fbshipit-source-id: 4b3a5e8a9071fcad65a19f193bf3dc7dc3ba1b96
Summary:
This PR helps with https://github.com/pytorch/pytorch/issues/50513 by reducing the complexity of our `mypy` test suite and making it easier to reproduce on the command line. Previously, to reproduce how `mypy` was actually run on tracked source files (ignoring the doctest typechecking) in CI, you technically needed to run 9 different commands with various arguments:
```
$ mypy --cache-dir=.mypy_cache/normal --check-untyped-defs --follow-imports silent
$ mypy --cache-dir=.mypy_cache/examples --follow-imports silent --check-untyped-defs test/type_hint_tests/module_list.py
$ mypy --cache-dir=.mypy_cache/examples --follow-imports silent --check-untyped-defs test/type_hint_tests/namedtuple.py
$ mypy --cache-dir=.mypy_cache/examples --follow-imports silent --check-untyped-defs test/type_hint_tests/opt_size.py
$ mypy --cache-dir=.mypy_cache/examples --follow-imports silent --check-untyped-defs test/type_hint_tests/size.py
$ mypy --cache-dir=.mypy_cache/examples --follow-imports silent --check-untyped-defs test/type_hint_tests/tensor_copy.py
$ mypy --cache-dir=.mypy_cache/examples --follow-imports silent --check-untyped-defs test/type_hint_tests/torch_cuda_random.py
$ mypy --cache-dir=.mypy_cache/examples --follow-imports silent --check-untyped-defs test/type_hint_tests/torch_optim.py
$ mypy --cache-dir=.mypy_cache/strict --config mypy-strict.ini
```
Now you only have to run 2 much simpler commands:
```
$ mypy
$ mypy --config mypy-strict.ini
```
One reason this is useful is because it will make it easier to integrate PyTorch's `mypy` setup into editors (remaining work on this to be done in a followup PR).
Also, as shown in the test plan, this also reduces the time it takes to run `test/test_type_hints.py` incrementally, by reducing the number of times `mypy` is invoked while still checking the same set of files with the same configs.
(Because this PR merges `test_type_hint_examples` (added in https://github.com/pytorch/pytorch/issues/34595) into `test_run_mypy` (added in https://github.com/pytorch/pytorch/issues/36584), I've added some people involved in those PRs as reviewers, in case there's a specific reason they weren't combined in the first place.)
Pull Request resolved: https://github.com/pytorch/pytorch/pull/50631
Test Plan:
Run this twice (the first time is to warm the cache):
```
$ python test/test_type_hints.py -v
```
- *Before:*
```
test_doc_examples (__main__.TestTypeHints)
Run documentation examples through mypy. ... ok
test_run_mypy (__main__.TestTypeHints)
Runs mypy over all files specified in mypy.ini ... ok
test_run_mypy_strict (__main__.TestTypeHints)
Runs mypy over all files specified in mypy-strict.ini ... ok
test_type_hint_examples (__main__.TestTypeHints)
Runs mypy over all the test examples present in ... ok
----------------------------------------------------------------------
Ran 4 tests in 5.090s
OK
```
You can also just run `mypy` to see how many files it checks:
```
$ mypy --cache-dir=.mypy_cache/normal --check-untyped-defs --follow-imports silent
Success: no issues found in 1192 source files
```
- *After:*
```
test_doc_examples (__main__.TestTypeHints)
Run documentation examples through mypy. ... ok
test_run_mypy (__main__.TestTypeHints)
Runs mypy over all files specified in mypy.ini ... ok
test_run_mypy_strict (__main__.TestTypeHints)
Runs mypy over all files specified in mypy-strict.ini ... ok
----------------------------------------------------------------------
Ran 3 tests in 2.404s
OK
```
Now `mypy` checks 7 more files, which is the number in `test/type_hint_tests`:
```
$ mypy
Success: no issues found in 1199 source files
```
Reviewed By: zou3519
Differential Revision: D25932660
Pulled By: samestep
fbshipit-source-id: 26c6f00f338e7b44954e5ed89522ce24e2fdc5f0
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/50611
Removed the unused old-style code to prevent it from being used.
Added all autograd/gen_pyi sources to mypy-strict.ini config.
Confirmed byte-for-byte compatible with the old codegen:
```
Run it before and after this PR:
.jenkins/pytorch/codegen-test.sh <baseline_output_dir>
.jenkins/pytorch/codegen-test.sh <test_output_dir>
Then run diff to compare the generated files:
diff -Naur <baseline_output_dir> <test_output_dir>
```
Confirmed clean mypy-strict run:
```
mypy --config mypy-strict.ini
```
Test Plan: Imported from OSS
Reviewed By: ezyang
Differential Revision: D25929730
Pulled By: ljk53
fbshipit-source-id: 1fc94436fd4a6b9b368ee0736e99bfb3c01d38ef
Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/49735
This is the final wave of autograd codegen data model migration.
After this PR:
- autograd codegen no longer depends on Declarations.yaml;
- autograd codegen sources are fully type annotated and pass mypy-strict check;
To avoid potential merge conflicts with other pending PRs, some structural
changes are intentionally avoided, e.g. didn't move inner methods out, didn't
change all inner methods to avoid reading outer function's variables, and etc.
Confirmed byte-for-byte compatible with the old codegen:
```
Run it before and after this PR:
.jenkins/pytorch/codegen-test.sh <baseline_output_dir>
.jenkins/pytorch/codegen-test.sh <test_output_dir>
Then run diff to compare the generated files:
diff -Naur <baseline_output_dir> <test_output_dir>
```
Confirmed clean mypy-strict run:
```
mypy --config mypy-strict.ini
```
Test Plan: Imported from OSS
Reviewed By: ezyang, bhosmer
Differential Revision: D25678879
Pulled By: ljk53
fbshipit-source-id: ba6e2eb6b9fb744208f7f79a922d933fcc3bde9f
