Opened 9 years ago
Closed 6 years ago
#10064 closed defect (fixed)
-1 in expression "is_positive".
Reported by: | fmaltey | Owned by: | burcin |
---|---|---|---|
Priority: | critical | Milestone: | sage-5.11 |
Component: | calculus | Keywords: | sd35 |
Cc: | robertwb, kcrisman | Merged in: | sage-5.11.beta1 |
Authors: | Mike Hansen | Reviewers: | Burcin Erocal |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #7160 | Stopgaps: |
Description (last modified by )
I test :
y = I*I*x / x # so y is the expression -1 y._is_positive() # the answer is True (and I dislike) z = -x / x z._is_positive() # the answer is False bool (z == y) # the answer is True
Attachments (2)
Change History (15)
comment:1 Changed 9 years ago by
- Milestone set to sage-4.6
comment:2 Changed 9 years ago by
- Cc robertwb added
- Priority changed from major to critical
- Status changed from new to needs_review
attachment:trac_10064-compare_I.patch changes the quadratic number field element comparison code to return a more sensible result. Now we have:
sage: m = (I*I).pyobject() sage: m > 0 False sage: m < 0 True
This also fixes #10062.
comment:3 Changed 9 years ago by
- Status changed from needs_review to needs_work
Here is a small bit of code that breaks (in 4.5.3) with trac_10064-compare_I.patch applied:
K.<a> = QuadraticField(5) E = EllipticCurve(K,[-1, -1, 1/2*a + 1/2, -1/2*a - 1/2, 1/2*a + 1/2]) E.torsion_subgroup()
comment:4 Changed 9 years ago by
Please see http://tinypaste.com/3f27d for the error from the above code snippet.
comment:5 Changed 9 years ago by
- Cc kcrisman added
comment:6 Changed 9 years ago by
Apparently related to #7160 and #6132, see this sage-devel discussion.
Changed 8 years ago by
comment:7 Changed 8 years ago by
- Dependencies set to 7160
- Status changed from needs_work to needs_review
I've added a patch which verifies that is fixed by #7160.
Apply trac_10064.patch
comment:8 Changed 8 years ago by
- Dependencies changed from 7160 to #7160
comment:9 Changed 8 years ago by
- Keywords sd35 added
comment:10 Changed 8 years ago by
- Description modified (diff)
- Milestone changed from sage-4.8 to sage-5.0
- Reviewers set to Burcin Erocal
- Status changed from needs_review to positive_review
Looks good to me.
comment:11 Changed 8 years ago by
- Milestone changed from sage-5.0 to sage-pending
Changed 6 years ago by
comment:12 Changed 6 years ago by
- Description modified (diff)
- Milestone changed from sage-pending to sage-5.11
comment:13 Changed 6 years ago by
- Merged in set to sage-5.11.beta1
- Resolution set to fixed
- Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
Simpler example:
The
is_positive()
test in pynac is at line 923 in numeric.cpp:We use the python comparison. Unfortunately, this doesn't work as expected for number field elements: