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: | sage-9.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: (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.
Change History (24)
comment:1 Changed 6 years ago by
- Dependencies set to pynac-0.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 pynac-0.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 sage-6.8 to sage-8.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 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.
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 sage-8.1 to sage-8.2
- Status changed from needs_work to needs_review
New commits:
1c62b17 | 18630: Doctest improvements to ex.is_positive/negative
|
comment:11 follow-up: ↓ 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**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
comment:12 follow-up: ↓ 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 ; follow-up: ↓ 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 pynac-0.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/18630-1
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: (1-pi).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 sage-8.2 to sage-9.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/18630-1 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 pynac-0.3.9.1/0.4.1)