Keep zero check be compatible with different sympy versions #130729
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/130729
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 0a837f7 with merge base 39eeaac ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
I guess we should enforce the latest sympy version only? |
#130689 intends to enforce to upgrade sympy version to 1.13 or later. |
|
@pytorchbot rebase -b main |
|
@pytorchbot started a rebase job onto refs/remotes/origin/main. Check the current status here |
|
Successfully rebased |
[ghstack-poisoned]
|
@pytorchbot rebase -b main |
|
@pytorchbot started a rebase job onto refs/remotes/origin/main. Check the current status here |
|
Successfully rebased |
|
@pytorchbot merge |
Merge failedReason: This PR needs a If not, please add the To add a label, you can comment to pytorchbot, for example For more information, see Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge |
Merge startedYour 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 |
| # Make unknown() * wrap(0) == wrap(0) | ||
| if a == 0: | ||
| # Make unknown() * wrap(0.0) == wrap(0.0) | ||
| if a == 0.0: |
There was a problem hiding this comment.
if a is 0 (integer) does this still match it?
There was a problem hiding this comment.
Yes
>>> import sympy
>>> a = sympy.Number(0)
>>> a == 0.0
True # for different sympy versionsThere was a problem hiding this comment.
It seems that is_zero is a better choice.
>>> import sympy
>>> a = sympy.Number(0)
>>> a.is_zero
True
>>> b = sympy.Number(0.0)
>>> b.is_zero
True# Motivation I found a difference between sympy 1.12 and 1.13. ```python # for 1.12 >>> import sympy >>> a = sympy.Number(0.0) >>> a == 0 True ``` ```python # for 1.13 >>> import sympy >>> a = sympy.Number(0.0) >>> a == 0 False ``` The different behavior will impact the result of [safe_mul](https://github.com/pytorch/pytorch/blob/6beec34b1c6803d5f6648c3cd7c262d6432374c8/torch/utils/_sympy/value_ranges.py#L521-L528), resulting in an incorrect results when `a = sympy.Number(0.0)`, `b = inf` and the result is `nan` if sympy version is 1.13. (the expected result is **0**) ```python def safe_mul(a, b): # Make unknown() * wrap(0.0) == wrap(0.0) if a == 0.0: return a elif b == 0.0: return b else: return a * b ``` In different sympy versions, `sympy.Number(0)` always has the same behavior that equals to 0.0. ```python >>> import sympy >>> a = sympy.Number(0) >>> a == 0.0 True # for different sympy versions ``` So, use 0.0 when checking zero in safe_mul to keep compatible with different sympy versions. Pull Request resolved: #130729 Approved by: https://github.com/lezcano, https://github.com/EikanWang
…130729) # Motivation I found a difference between sympy 1.12 and 1.13. ```python # for 1.12 >>> import sympy >>> a = sympy.Number(0.0) >>> a == 0 True ``` ```python # for 1.13 >>> import sympy > 9E88 ;>> a = sympy.Number(0.0) >>> a == 0 False ``` The different behavior will impact the result of [safe_mul](https://github.com/pytorch/pytorch/blob/6beec34b1c6803d5f6648c3cd7c262d6432374c8/torch/utils/_sympy/value_ranges.py#L521-L528), resulting in an incorrect results when `a = sympy.Number(0.0)`, `b = inf` and the result is `nan` if sympy version is 1.13. (the expected result is **0**) ```python def safe_mul(a, b): # Make unknown() * wrap(0.0) == wrap(0.0) if a == 0.0: return a elif b == 0.0: return b else: return a * b ``` In different sympy versions, `sympy.Number(0)` always has the same behavior that equals to 0.0. ```python >>> import sympy >>> a = sympy.Number(0) >>> a == 0.0 True # for different sympy versions ``` So, use 0.0 when checking zero in safe_mul to keep compatible with different sympy versions. Pull Request resolved: pytorch#130729 Approved by: https://github.com/lezcano, https://github.com/EikanWang
…130729) # Motivation I found a difference between sympy 1.12 and 1.13. ```python # for 1.12 >>> import sympy >>> a = sympy.Number(0.0) >>> a == 0 True ``` ```python # for 1.13 >>> import sympy >>> a = sympy.Number(0.0) >>> a == 0 False ``` The different behavior will impact the result of [safe_mul](https://github.com/pytorch/pytorch/blob/6beec34b1c6803d5f6648c3cd7c262d6432374c8/torch/utils/_sympy/value_ranges.py#L521-L528), resulting in an incorrect results when `a = sympy.Number(0.0)`, `b = inf` and the result is `nan` if sympy version is 1.13. (the expected result is **0**) ```python def safe_mul(a, b): # Make unknown() * wrap(0.0) == wrap(0.0) if a == 0.0: return a elif b == 0.0: return b else: return a * b ``` In different sympy versions, `sympy.Number(0)` always has the same behavior that equals to 0.0. ```python >>> import sympy >>> a = sympy.Number(0) >>> a == 0.0 True # for different sympy versions ``` So, use 0.0 when checking zero in safe_mul to keep compatible with different sympy versions. Pull Request resolved: pytorch#130729 Approved by: https://github.com/lezcano, https://github.com/EikanWang
Stack from ghstack (oldest at bottom):
Motivation
I found a difference between sympy 1.12 and 1.13.
The different behavior will impact the result of safe_mul, resulting in an incorrect results when
a = sympy.Number(0.0),b = infand the result isnanif sympy version is 1.13. (the expected result is 0)In different sympy versions,
sympy.Number(0)always has the same behavior that equals to 0.0.So, use 0.0 when checking zero in safe_mul to keep compatible with different sympy versions.
cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10