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

Doctest: Expression.is_positive/negative fixed #18630

Closed
rwst opened this issue Jun 7, 2015 · 33 comments
Closed

Doctest: Expression.is_positive/negative fixed #18630

rwst opened this issue Jun 7, 2015 · 33 comments

Comments

@rwst
Copy link

rwst commented Jun 7, 2015

The two functions that query Pynac expressions' info::flags only work with numerics and symbols with domain. The rest simply returns False:

sage: (1-pi).is_negative()
False
sage: (log(1/2)).is_negative()
False
sage: e.is_positive()
False
sage: (e+1).is_positive()
False
sage: (2*e).is_positive()
False
sage: (e^3).is_positive()
False

UPDATE: everything above but the first works now.

Depends on #24497

CC: @kcrisman @pelegm

Component: symbolics

Author: Ralf Stephan

Branch/Commit: 8e446e6

Reviewer: Peleg Michaeli, Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/18630

@rwst rwst added this to the sage-6.8 milestone Jun 7, 2015
@rwst
Copy link
Author

rwst commented Jun 9, 2015

Changed upstream from Not yet reported upstream; Will do shortly. to Fixed upstream, but not in a stable release.

@rwst
Copy link
Author

rwst commented Jun 9, 2015

comment:1

Parts of this are fixed in pynac/pynac#66 (will be pynac-0.3.9.1/0.4.1)

@rwst
Copy link
Author

rwst commented Jun 9, 2015

Dependencies: pynac-0.3.9.1

@rwst
Copy link
Author

rwst commented Jun 10, 2015

comment:2

While products are okay already, sums with negative terms are not and will not be for some time. Correct handling depends on pynac/pynac#67 and pynac/pynac#68 and affects #12588 too, for example.

@rwst

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Jul 10, 2015

Changed upstream from Fixed upstream, but not in a stable release. to Reported upstream. Developers acknowledge bug.

@rwst
Copy link
Author

rwst commented Jul 10, 2015

Changed dependencies from pynac-0.3.9.1 to none

@rwst
Copy link
Author

rwst commented Jun 7, 2017

comment:4

Since everything apart from sums works now, let's look at what is possible with sums containing exact numerics (in the Pynac sense) and constants only. The border case is if the expression is nonzero---but only at certain precision. This is impossible to handle. It is also not possible to convert to float and only check sign if the absolute result is greater some epsilon, because that could be due to precision loss in a denominator.

I'm therefore just adding the doctests here to resolve the ticket.

@rwst
Copy link
Author

rwst commented Jun 7, 2017

Changed upstream from Reported upstream. Developers acknowledge bug. to none

@rwst rwst modified the milestones: sage-6.8, sage-8.1 Jun 7, 2017
@rwst rwst changed the title Expression.is_positive/negative incomplete Doctest: Expression.is_positive/negative fixed Jun 7, 2017
@rwst
Copy link
Author

rwst commented Jun 7, 2017

@rwst
Copy link
Author

rwst commented Jun 7, 2017

New commits:

fe039e218630: Doctest: Expression.is_positive/negative fixed

@rwst
Copy link
Author

rwst commented Jun 7, 2017

Commit: fe039e2

@rwst
Copy link
Author

rwst commented Jun 7, 2017

Author: Ralf Stephan

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 9, 2017

Branch pushed to git repo; I updated commit sha1. New commits:

a643c39Merge branch 'develop' into t/18630/expression_is_positive_negative_incomplete

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 9, 2017

Changed commit from fe039e2 to a643c39

@rwst
Copy link
Author

rwst commented Nov 28, 2017

comment:8

Docbuild error: ValueError: line 48 of the docstring for sage.symbolic.expression.Expression.is_positive has inconsistent leading whitespace: u'::'

@rwst
Copy link
Author

rwst commented Dec 4, 2017

Changed branch from u/rws/expression_is_positive_negative_incomplete to u/rws/18630

@rwst
Copy link
Author

rwst commented Dec 4, 2017

New commits:

1c62b1718630: Doctest improvements to ex.is_positive/negative

@rwst
Copy link
Author

rwst commented Dec 4, 2017

Changed commit from a643c39 to 1c62b17

@rwst rwst removed this from the sage-8.1 milestone Dec 4, 2017
@rwst rwst added this to the sage-8.2 milestone Dec 4, 2017
@videlec
Copy link
Contributor

videlec commented Dec 6, 2017

comment:11

Would be nice to also have some less trivial examples

sage: (log(1/3) * log(1/2)).is_positive()
True
sage: log((2**500+1)/2**500).is_positive()
True
sage: log(2*500/(2**500-1)).is_negative()
True

And the following are other failing examples

sage: sin(pi/37).is_positive()   # algebraic number!
False
sage: (sqrt(2)-1).is_positive()  # algebric number!
False
sage: log(pi).is_positive()
False 
sage: sin(3).is_positive()
False
sage: sin(5).is_negative()
False

@videlec
Copy link
Contributor

videlec commented Dec 6, 2017

comment:12

And I don't understand why the following is ok

sage: ((-pi^(1/5))^2).is_positive()
True

while not these ones

sage: (pi^2).is_positive()
False
sage: ((-pi)^2).is_positive()
False

@rwst
Copy link
Author

rwst commented Dec 7, 2017

comment:13

Replying to @videlec:

And the following are other failing examples

sage: sin(pi/37).is_positive()   # algebraic number!
False
sage: (sqrt(2)-1).is_positive()  # algebric number!
False
sage: log(pi).is_positive()
False 
sage: sin(3).is_positive()
False
sage: sin(5).is_negative()
False

They are not failing. The result False simply means "I don't know.".

@rwst
Copy link
Author

rwst commented Dec 7, 2017

comment:14

Replying to @videlec:

sage: (pi^2).is_positive()
False
sage: ((-pi)^2).is_positive()
False

One logic for b^e.is_positive() is if (e.is_even()) return b.is_real() and b.is_nonzero() but is_nonzero was not implemented for constants until now. This is fixed in pynac/pynac@e92febc and will be in pynac-0.7.15. We can as well put all of these doctests in this ticket too. Thanks for the report!

@rwst
Copy link
Author

rwst commented Dec 7, 2017

comment:15

Replying to @rwst:

They are not failing. The result False simply means "I don't know.".

See also #22162.

@rwst
Copy link
Author

rwst commented Dec 9, 2017

comment:16

See also pynac/pynac#293 for progress on what is completely implemented as internal Pynac flag.

@rwst
Copy link
Author

rwst commented Jan 27, 2018

Dependencies: #24497

@rwst
Copy link
Author

rwst commented Jan 29, 2018

Changed branch from u/rws/18630 to u/rws/18630-1

@rwst
Copy link
Author

rwst commented Jan 29, 2018

Changed commit from 1c62b17 to 8e446e6

@rwst
Copy link
Author

rwst commented Jan 29, 2018

New commits:

8e446e618630: Doctest improvements to ex.is_positive/negative

@pelegm
Copy link
Contributor

pelegm commented Jun 30, 2018

comment:21

Seems like the first test still fails:

sage: (1-pi).is_negative()
False

Do we want to merge it as is?

@rwst
Copy link
Author

rwst commented Jun 30, 2018

comment:22

Yes, see comment:15.

@videlec
Copy link
Contributor

videlec commented Feb 16, 2020

Reviewer: Peleg Michaeli, Vincent Delecroix

@vbraun
Copy link
Member

vbraun commented Feb 19, 2020

Changed branch from u/rws/18630-1 to 8e446e6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants