Skip to content

Conversation

@pytorch-bot
Copy link

pytorch-bot bot commented Nov 3, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 559876f with merge base e6ff07f (image):
💚 Looks good so far! There are no failures yet. 💚

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

cc ezyang SherlockNoMad EikanWang jgong5 wenzhe-nrv

[ghstack-poisoned]
cc ezyang SherlockNoMad EikanWang jgong5 wenzhe-nrv

[ghstack-poisoned]
cc ezyang SherlockNoMad EikanWang jgong5 wenzhe-nrv

[ghstack-poisoned]
@bobrenjc93 bobrenjc93 added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 4, 2024
cc ezyang SherlockNoMad EikanWang jgong5 wenzhe-nrv

[ghstack-poisoned]
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

I'm not sure if this covers all cases (maybe you can argue why it does) but it seems harmless to start. Specifically, the potential soundness problem is if the suppressed FX node ends up producing a guard which the symbolic reasoning system relies on to do a simplification; TV will now incorrectly say that the original guards (which are missing this guard) don't entail the final ones. But I don't think we should ever rely on float guards in this case.

cc ezyang SherlockNoMad EikanWang jgong5 wenzhe-nrv

[ghstack-poisoned]
@bobrenjc93
Copy link
Contributor Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@github-actions github-actions bot deleted the gh/bobrenjc93/105/head branch December 5, 2024 02:16
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
)

Fixes the test `python test/inductor/test_torchinductor.py CpuTests.test_upsample_cat_conv_cpu` when `specialize_float=False`

Pull Request resolved: pytorch#139454
Approved by: https://github.com/ezyang
ghstack dependencies: pytorch#139569, pytorch#139457, pytorch#139568, pytorch#139572, pytorch#139846
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
Fixes `PYTORCH_OPINFO_SAMPLE_INPUT_INDEX=7 python test/inductor/test_torchinductor_opinfo.py TestInductorOpInfoCPU.test_comprehensive_linalg_norm_cpu_float16` when `specialize_float=False`

Pull Request resolved: pytorch#139896
Approved by: https://github.com/ezyang
ghstack dependencies: pytorch#139569, pytorch#139457, pytorch#139568, pytorch#139572, pytorch#139846, pytorch#139454
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
…139935)

Fixes `PYTORCH_TEST_WITH_DYNAMO=1 python test/test_torch.py TestTorchDeviceTypeCPU.test_gradient_type_promotion_cpu` when `specialize_float=False`

Reviewers might wonder why we need to have this whitelist. Can't we rely on python_arg_parser.h to do the specialization generically? Alas this path doesn't actually FFI to C++ so we do need to do the specialization in pythonland.

Pull Request resolved: pytorch#139935
Approved by: https://github.com/ezyang
ghstack dependencies: pytorch#139569, pytorch#139457, pytorch#139568, pytorch#139572, pytorch#139846, pytorch#139454, pytorch#139896
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
…h#139587)

This fix was a bit more involved:
1) It fixes a item_memo loss place.
2) It updates a test to be eager instead of aot_eager since it reveals a very obscure bug related to replacements that's not worth solving since in practice inductor will regenerate the runtime asserts anyways
3) It updates tensorify to specialize more places now that the aforementioned bug is fixed.

Fixes `PYTORCH_OPINFO_SAMPLE_INPUT_INDEX=6 python test/inductor/test_torchinductor_opinfo.py TestInductorOpInfoCPU.test_comprehensive_linalg_norm_cpu_float16` when `specialize_float=False`

while ensuring `python test/dynamo/test_dynamic_shapes.py DynamicShapesMiscTests.test_runtime_assert_replacement_dynamic_shapes` doesn't regress

Pull Request resolved: pytorch#139587
Approved by: https://github.com/ezyang
ghstack dependencies: pytorch#139569, pytorch#139457, pytorch#139568, pytorch#139572, pytorch#139846, pytorch#139454, pytorch#139896, pytorch#139935
pobin6 pushed a commit to pobin6/pytorch that referenced this pull request Dec 5, 2024
Fixes `PYTORCH_TEST_WITH_INDUCTOR=1 tlp python test/test_torch.py TestTorchDeviceTypeCUDA.test_cauchy_cuda_float64` when specialize_float=False

Pull Request resolved: pytorch#139583
Approved by: https://github.com/ezyang
ghstack dependencies: pytorch#139569, pytorch#139457, pytorch#139568, pytorch#139572, pytorch#139846, pytorch#139454, pytorch#139896, pytorch#139935, pytorch#139587
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request fx Merged release notes: fx release notes category topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants