Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated assumptions: Now complex and !finite infers infinite. #16592

Merged
merged 4 commits into from Apr 19, 2019

Conversation

ShubhamKJha
Copy link
Member

@ShubhamKJha ShubhamKJha commented Apr 8, 2019

References to other Issues or PRs

Fixes #16579

Brief description of what is fixed or changed

A complex value can either be finite or infinite. Therefore, complex and not finite should mean the value is infinite. Currently, SymPy doesn't account for this.
With this PR,

>>> from sympy import Dummy
>>> Dummy(complex=True, finite=False).is_infinite
True

Other comments

Release Notes

  • core
    • added new clause complex -> finite | infinite in _assume_rules in assumptions.py.

@sympy-bot
Copy link

sympy-bot commented Apr 8, 2019

Hi, I am the SymPy bot (v147). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

  • core
    • added new clause complex -> finite | infinite in _assume_rules in assumptions.py. (#16592 by @ShubhamKJha)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.5.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234". See
https://github.com/blog/1506-closing-issues-via-pull-requests . Please also
write a comment on that issue linking back to this pull request once it is
open. -->
Fixes #16579 

#### Brief description of what is fixed or changed
A complex value can either be `finite` or `infinite`. Therefore, `complex and not finite` should mean the value is `infinite`. Currently, SymPy doesn't account for this. 
With this PR,
```py
>>> from sympy import Dummy
>>> Dummy(complex=True, finite=False).is_infinite
True
```

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* core
  * added new clause `complex  ->   finite | infinite` in `_assume_rules` in assumptions.py.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@codecov
Copy link

codecov bot commented Apr 8, 2019

Codecov Report

Merging #16592 into master will increase coverage by 0.027%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##            master    #16592       +/-   ##
=============================================
+ Coverage   73.805%   73.833%   +0.027%     
=============================================
  Files          619       619               
  Lines       159230    159230               
  Branches     37370     37370               
=============================================
+ Hits        117521    117565       +44     
+ Misses       36273     36236       -37     
+ Partials      5436      5429        -7

@jksuom
Copy link
Member

jksuom commented Apr 8, 2019

Please add a test.

@oscargus
Copy link
Contributor

oscargus commented Apr 8, 2019

I'm not that well versed with the assumption system, will this also mean that the same thing hold for e.g. real?

(Wouldn't it be enough to add an assumption as finite == !infinite? Which I realize that there already is one. So this is really the connection that any number that is finite is not infinite and vice versa? Maybe the release note should be updated to clarify this?)

@ShubhamKJha
Copy link
Member Author

HI @oscargus we can't take finite == !infinite, there can numbers which are both not finite and not infinite (e.g. nan). The case that finite infers not infinite holds for complex numbers (real numbers are complex so they are covered).
How should I update release notes?

@oscargus
Copy link
Contributor

oscargus commented Apr 8, 2019

OK! Yeah, now I noted that it is only in one direction, so infinite -> !finite, not the other way around.

I'm just thinking that maybe one should focus on the practical effect in the release note. Now, if you specify any property of the number (complex, imaginary, real, rational, integer) and either finite or infinite, it will inherit the other finite/infinite property. This is clearly not how it should be formulated though... But more the practical consequence than the code change.

This also means that specifying e.g. real means that it cannot take on NaN by definition and that is really the only difference between that and not specifying anything.

It leads to the question: what does this really say about x?

Symbol('x', finite=False, infinite=False)

xis NaN?

@ShubhamKJha
Copy link
Member Author

ShubhamKJha commented Apr 8, 2019

It leads to the question: what does this really say about x?

Symbol('x', finite=False, infinite=False)

xis NaN?

Now I see, nan.is_finite and nan.is_infinite both return None. Though clearly there can be consequences of having finite == !infinite, I can't find an example which breaks this. Another argument is in this commit's message.

@asmeurer
Copy link
Member

I guess I originally wanted to add complex -> finite in #8293, but it was too hard. This is a good step forward.

We should decide once and for all however exactly which predicates should contain infinity.

@@ -1062,3 +1062,10 @@ def test_issue_16313():
assert (l*x).is_real is False
assert (l*x*x).is_real is None # since x*x can be a real number
assert (-x).is_positive is False

def test_issue_16579():
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a comment here that complex may not imply infinite in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I would add comment.

@ShubhamKJha
Copy link
Member Author

@jksuom is it good to be merged?

@jksuom
Copy link
Member

jksuom commented Apr 19, 2019

With this complex and not finite would infer infinite. Since reals are complex, the implication follows in real as well.

I would remove this explanation from the release notes. Maybe it, or part of it, could go in the comments above. Otherwise I think this is good to be merged.

@ShubhamKJha
Copy link
Member Author

Hi @jksuom considering the work in #16603 the scenario with reals may change so I am not adding this to comments. I revised the release notes please merge it.

@jksuom
Copy link
Member

jksuom commented Apr 19, 2019

Thanks.

@jksuom jksuom merged commit 967df3b into sympy:master Apr 19, 2019
@ShubhamKJha ShubhamKJha deleted the new_fix_assumptions branch May 24, 2019 08:12
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.

What should be the definition of infinite?
5 participants