Skip to content

Conversation

@eqy
Copy link
Collaborator

@eqy eqy commented Apr 6, 2023

mlazos and others added 30 commits March 22, 2023 08:30
Summary: Have minifier include the current buck target as a dependency to make sure all deps are included.

Test Plan: TORCH_COMPILE_DEBUG_DIR=”.” buck2 run mode/dev-nosan //caffe2/test/inductor:minifier_smoke

Differential Revision: D44231209

Pull Request resolved: #97183
Approved by: https://github.com/anijain2305
If python development library is missing when building pytorch from source, cmake will raise the error like:
```
CMake Error at cmake/Dependencies.cmake:1079 (if):
  if given arguments:

    "VERSION_LESS" "3"

  Unknown arguments specified
```

it's quite a misleading information that user would consider it's a syntax error or cmake version problem.

This PR add a check to ensure `PYTHONLIBS_VERSION_STRING` exist before using.

Related  #87993

Pull Request resolved: #96642
Approved by: https://github.com/kit1980
…rmance (#96954)

As title, enable mkldnn packed linear to improve bfloat16 performance.

Pull Request resolved: #96954
Approved by: https://github.com/EikanWang, https://github.com/jgong5, https://github.com/desertfire
Summary: Log the generated code for those two flaky tests to see if
there is any codegen difference when they fail.

Pull Request resolved: #97307
Approved by: https://github.com/ezyang
Signed-off-by: Edward Z. Yang <ezyang@meta.com>

Pull Request resolved: #97309
Approved by: https://github.com/janeyx99, https://github.com/zou3519
remove dead proto_convert library

Summary:
This has no code, only a collection of headers. Just make sure the
only thing that includes it still builds.

Test Plan: Rely on CI.

Reviewers: sahanp

Subscribers:

Tasks:

Tags:

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/pytorch/pytorch/pull/97322).
* #97337
* #97336
* #97335
* #97334
* #97325
* #97324
* #97323
* __->__ #97322
Pull Request resolved: #97322
Approved by: https://github.com/malfet
…tributes (#96933)

**Summary** NamedTuple attributes can be annotated to declare their type:
```python
class MyNamedTuple(NamedTuple):
    x: int
    y: torch.Tensor
    z: MyOtherType
```
Normally in python you can also declare your types as strings, `x: 'int'`. But NamedTuples previously didn't support this, because their annotation evaluation process was slightly different. This PR updates the NamedTuple attribute type annotation evaluation method to support ForwardRef declarations (i.e. declaring as strings).

**Details**

Below I repeat the comment I left in _jit_internal.py:

NamedTuple types are slightly different from normal types.

Normally, annotations are evaluted like this (during jit.script):
1. Load strings of python code into c++ and parse.
2. Get annotations as strings
3. Use the PythonResolver's resolution callback (rcb) to convert the string into a python object
4. We call into annotations.py:ann_to_type to convert python obj from step 3 into a type that torchscript understands.

NamedTuples are more complicated, because they have sub-types. Normally, once we have the NamedTuple type object from #3, we can just look at the annotation literal values and use ann_to_type directly on them.

But sometimes, users will annotate with string literals, e.g.
```
   x: 'int'
```
This also happens with PEP563 (from __forward__ import annotations)

These annotations appear in the annotation dict as ForwardRef('int').

Then, we need to convert the string into a python object. This requires having local context for custom objects or imported types. rcb() is what gives us this. So, we plumb rcb through the stack so it can be used in this context for the if block below.

FAQ:
- Why do we need this special handling for NamedTuple but string annotations work fine for normal types? Normally, we parse the string directly and then call rcb() directly from C++.
- Why not use ForwardRef._evaluate? For that, we need globals() and locals() for the local context where the NamedTuple was defined. rcb is what lets us look up into these. So, basically rcb does the hard work for us.
- What is rcb? rcb is a ResolutionCallback - python callable that takes a string and returns a type. It's generated by `createResolutionCallback.*` in _jit_internal.py.

**Why is this only partial support**:

This only plumbs the rcb through some paths. In particular, the `toSugaredValue` path uses a fake rcb.

**Alternatives**:

We could also treat this the way we treat non-nn.Module classes: we evaluate them separately, ahead of time. That solution is probably better, but probably requires a more risky refactor for the way NamedTuples are handled.

Fixes #95858

Pull Request resolved: #96933
Approved by: https://github.com/qihqi
Seen first in error message:
```
[2023-03-22 10:30:39,786] torch._dynamo.convert_frame: [WARNING] torch._dynamo hit config.cache_size_limit (64)
   function: '<resume in paste_mask_in_image>' (/vision/torchvision/models/detection/roi_heads.py:407)
   reasons:  w == 857
to diagnose recompilation issues, see https://pytorch.org/docs/master/dynamo/troubleshooting.html.
[2023-03-22 10:30:40,036] torch._dynamo.convert_frame: [WARNING] torch._dynamo hit config.cache_size_limit (64)
   function: '<resume in paste_mask_in_image>' (/vision/torchvision/models/detection/roi_heads.py:406)
   reasons:  ___stack0 == 207
to diagnose recompilation issues, see https://pytorch.org/docs/master/dynamo/troubleshooting.html.
```

Broken link:
- https://pytorch.org/docs/master/dynamo/troubleshooting.html.

Good link:
- https://pytorch.org/docs/master/compile/troubleshooting.html

Pull Request resolved: #97330
Approved by: https://github.com/zou3519
Updates:
- ~recommend user to use non-reentrant, mention that reentrant will be deprecated in the future~
- merges all the warnings into a single list of non-reentrant improvements over reentrant
- adds an additional entry to the list about allowing backward inside checkpointed region

Pull Request resolved: #96862
Approved by: https://github.com/albanD
remove dead torch_pb.h library

Summary: This is only used in one place, ensure it still builds.

Test Plan: Rely on CI.

Reviewers: sahanp

Subscribers:

Tasks:

Tags:

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/pytorch/pytorch/pull/97323).
* #97337
* #97336
* #97335
* #97334
* #97325
* #97324
* __->__ #97323
* #97322
Pull Request resolved: #97323
Approved by: https://github.com/malfet
move caffe2/proto/ to its own Bazel package

Summary:
This is just to break up build files and make the system easier to
reason about during the transition to the common build system.

Test Plan: Verified locally and rely on CI.

Reviewers: sahanp

Subscribers:

Tasks:

Tags:

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/pytorch/pytorch/pull/97324).
* #97337
* #97336
* #97335
* #97334
* #97325
* __->__ #97324
* #97323
* #97322
Pull Request resolved: #97324
Approved by: https://github.com/malfet
…ng (#81862)"

This reverts commit 701cdbb.

Reverted #81862 on behalf of https://github.com/facebook-github-bot due to Diff reverted internally
…on deallocation of live tensors (#97168)

Refining the logic for when it is okay to ignore previously live outputs from cudagraphs. If there is a forward that has been invoked without invocation of the corresponding backwards, dont allow overwriting outputs.

Differential Revision: [D44228369](https://our.internmc.facebook.com/intern/diff/D44228369)
Pull Request resolved: #97168
Approved by: https://github.com/ezyang, https://github.com/jansel
Summary:
Calls to this function without an argument will get a stack trace at
import time. This is expensive, we can just skip it by passing in a value.

Test Plan: Wait for tests

Differential Revision: D44244345

Pull Request resolved: #97274
Approved by: https://github.com/kiukchung
This reverts commit aa3a57b.

Reverted #97275 on behalf of https://github.com/ezyang due to this broke a test
Closes #87365

I added `as_strided_` to the tensor docs, following what seemed to be a pattern consistent with similar functions. More specifically, both the in-place and out-of-place function counterparts are defined in `_tensor_docs.py`, with the in-place version linking to the out-of-place version and the out-of-place version pointing to the corresponding `_torch_docs.py` definition.

If the above is not what we want (e.g. we want to add a more robust description, examples, etc.), let me know and I will be happy to update accordingly!

Pull Request resolved: #97300
Approved by: https://github.com/zou3519
This PR fixes incorrect schema for `minimum_value` in creating a primitive operation.

This PR also fixes typo in comment and python doc.

Pull Request resolved: #97327
Approved by: https://github.com/zou3519
#96866)

Why did I choose context manager instead of per-call? Early stopping is not part of the model definition, and depending on how a particular model is used, e.g., with PT2 or not we may or may not want to disable early stopping.
Pull Request resolved: #96866
Approved by: https://github.com/albanD
Change variable spelling from `need_atten_weights` to `need_attn_weights` to match naming convention elsewhere in pytorch.

Pull Request resolved: #97102
Approved by: https://github.com/drisspg
Per title, I will update the runbook to point to this after the review
Pull Request resolved: #97045
Approved by: https://github.com/clee2000, https://github.com/ZainRizvi
…7305)

This follows #96199 and supports the 'other' profiler.

Pull Request resolved: #97305
Approved by: https://github.com/voznesenskym
@pytorch-bot
Copy link

pytorch-bot bot commented Apr 6, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/98526

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

❌ 1 Failures

As of commit 7e8b4ee:

BROKEN TRUNK - The following jobs failed but were present on the merge base 08f125b:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@github-actions github-actions bot added ciflow/inductor module: amp (automated mixed precision) autocast module: cpu CPU specific problem (e.g., perf, algorithm) module: dynamo module: inductor module: mkldnn Related to Intel IDEEP or oneDNN (a.k.a. mkldnn) integration NNC release notes: quantization release notes category labels Apr 6, 2023
@eqy eqy closed this Apr 6, 2023
@pytorch-bot pytorch-bot bot added ciflow/mps Run MPS tests (subset of trunk) release notes: releng release notes category labels Apr 6, 2023
@IvanYashchuk IvanYashchuk removed their request for review April 7, 2023 06:50
@github-actions github-actions bot deleted the eqy-patch-13 branch September 28, 2024 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/inductor ciflow/mps Run MPS tests (subset of trunk) module: amp (automated mixed precision) autocast module: cpu CPU specific problem (e.g., perf, algorithm) module: dynamo module: inductor module: mkldnn Related to Intel IDEEP or oneDNN (a.k.a. mkldnn) integration NNC release notes: quantization release notes category release notes: releng release notes category

Projects

None yet

Development

Successfully merging this pull request may close these issues.