Opened 6 years ago
Closed 14 months ago
#18630 closed defect (fixed)
Doctest: Expression.is_positive/negative fixed
Reported by:  rws  Owned by:  

Priority:  major  Milestone:  sage9.1 
Component:  symbolics  Keywords:  
Cc:  kcrisman, pelegm  Merged in:  
Authors:  Ralf Stephan  Reviewers:  Peleg Michaeli, Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  8e446e6 (Commits, GitHub, GitLab)  Commit:  8e446e6360353057553e702f188166a7a0817a1e 
Dependencies:  #24497  Stopgaps: 
Description (last modified by )
The two functions that query Pynac expressions' info::flags
only work with numeric
s and symbol
s with domain. The rest simply returns False
:
sage: (1pi).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.
Change History (24)
comment:1 Changed 6 years ago by
 Dependencies set to pynac0.3.9.1
 Report Upstream changed from Not yet reported upstream; Will do shortly. to Fixed upstream, but not in a stable release.
comment:2 Changed 6 years ago by
While products are okay already, sums with negative terms are not and will not be for some time. Correct handling depends on https://github.com/pynac/pynac/issues/67 and https://github.com/pynac/pynac/issues/68 and affects #12588 too, for example.
comment:3 Changed 6 years ago by
 Dependencies pynac0.3.9.1 deleted
 Description modified (diff)
 Report Upstream changed from Fixed upstream, but not in a stable release. to Reported upstream. Developers acknowledge bug.
comment:4 Changed 4 years ago by
 Milestone changed from sage6.8 to sage8.1
 Report Upstream changed from Reported upstream. Developers acknowledge bug. to N/A
 Summary changed from Expression.is_positive/negative incomplete to Doctest: Expression.is_positive/negative fixed
Since everything apart from sums works now, let's look at what is possible with sums containing exact numeric
s (in the Pynac sense) and constants only. The border case is if the expression is nonzerobut 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.
comment:5 Changed 4 years ago by
 Branch set to u/rws/expression_is_positive_negative_incomplete
comment:6 Changed 4 years ago by
 Commit set to fe039e29d4e8c45c950722308b35d0188bccfcc3
 Status changed from new to needs_review
New commits:
fe039e2  18630: Doctest: Expression.is_positive/negative fixed

comment:7 Changed 3 years ago by
 Commit changed from fe039e29d4e8c45c950722308b35d0188bccfcc3 to a643c393571f00b2243d141425920250359da55b
Branch pushed to git repo; I updated commit sha1. New commits:
a643c39  Merge branch 'develop' into t/18630/expression_is_positive_negative_incomplete

comment:8 Changed 3 years ago by
 Status changed from needs_review to needs_work
Docbuild error: ValueError: line 48 of the docstring for sage.symbolic.expression.Expression.is_positive has inconsistent leading whitespace: u'::'
comment:9 Changed 3 years ago by
 Branch changed from u/rws/expression_is_positive_negative_incomplete to u/rws/18630
comment:10 Changed 3 years ago by
 Commit changed from a643c393571f00b2243d141425920250359da55b to 1c62b17993d9894a8b2beee20a63c985665b9304
 Milestone changed from sage8.1 to sage8.2
 Status changed from needs_work to needs_review
New commits:
1c62b17  18630: Doctest improvements to ex.is_positive/negative

comment:11 followup: ↓ 13 Changed 3 years ago by
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**5001)).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
comment:12 followup: ↓ 14 Changed 3 years ago by
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
comment:13 in reply to: ↑ 11 ; followup: ↓ 15 Changed 3 years ago by
Replying to vdelecroix:
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.".
comment:14 in reply to: ↑ 12 Changed 3 years ago by
 Status changed from needs_review to needs_work
Replying to vdelecroix:
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 https://github.com/pynac/pynac/commit/e92febc468800e4a1457f944bb7a45e9baf0e2f6 and will be in pynac0.7.15. We can as well put all of these doctests in this ticket too. Thanks for the report!
comment:15 in reply to: ↑ 13 Changed 3 years ago by
comment:16 Changed 3 years ago by
See also https://github.com/pynac/pynac/issues/293 for progress on what is completely implemented as internal Pynac flag.
comment:17 Changed 3 years ago by
 Dependencies set to #24497
comment:18 Changed 3 years ago by
 Branch changed from u/rws/18630 to u/rws/186301
comment:19 Changed 3 years ago by
 Commit changed from 1c62b17993d9894a8b2beee20a63c985665b9304 to 8e446e6360353057553e702f188166a7a0817a1e
 Status changed from needs_work to needs_review
New commits:
8e446e6  18630: Doctest improvements to ex.is_positive/negative

comment:20 Changed 3 years ago by
 Cc pelegm added
comment:21 Changed 3 years ago by
Seems like the first test still fails:
sage: (1pi).is_negative() False
Do we want to merge it as is?
comment:22 Changed 3 years ago by
Yes, see comment:15.
comment:23 Changed 14 months ago by
 Milestone changed from sage8.2 to sage9.1
 Reviewers set to Peleg Michaeli, Vincent Delecroix
 Status changed from needs_review to positive_review
comment:24 Changed 14 months ago by
 Branch changed from u/rws/186301 to 8e446e6360353057553e702f188166a7a0817a1e
 Resolution set to fixed
 Status changed from positive_review to closed
Parts of this are fixed in https://github.com/pynac/pynac/issues/66 (will be pynac0.3.9.1/0.4.1)