Opened 10 years ago
Closed 9 years ago
#11155 closed defect (fixed)
abs(pi+I) = pi+I
Reported by: | benreynwar | Owned by: | burcin |
---|---|---|---|
Priority: | critical | Milestone: | sage-5.1 |
Component: | symbolics | Keywords: | |
Cc: | burcin | Merged in: | sage-5.1.beta4 |
Authors: | Alexei Sheplyakov, Titus Nicolae | Reviewers: | Burcin Erocal |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #12950 | Stopgaps: |
Description (last modified by )
abs(pi+I) returns pi+I
one would have expected:
sqrt(pi^2+1)
Attachments (1)
Change History (12)
comment:1 Changed 10 years ago by
- Component changed from PLEASE CHANGE to symbolics
- Owner changed from tbd to burcin
- Priority changed from minor to major
comment:2 Changed 9 years ago by
- Cc burcin added
- Description modified (diff)
- Milestone set to sage-5.0
- Priority changed from major to critical
I can confirm this further. It's mathematically seriously incorrect... so now moving from major to critical, and cc'ing burcin.
Note:
sage: z = pi + i sage: abs(z) pi + I sage: sqrt((z*z.conjugate()).expand()) sqrt(pi^2 + 1)
comment:3 Changed 9 years ago by
Reported to the ginac-devel list:
http://thread.gmane.org/gmane.comp.mathematics.ginac.devel/1363
More detailed about the problem are in my message linked above.
comment:4 Changed 9 years ago by
Apparently already fixed!
comment:5 Changed 9 years ago by
Here is the diff.
-
check/exam_numeric.cpp
diff --git a/check/exam_numeric.cpp b/check/exam_numeric.cpp index 715acff..d5ae27b 100644 (file)
a b static unsigned exam_numeric1() 68 68 << " erroneously not recognized as complex rational" << endl; 69 69 ++result; 70 70 } 71 if (test_crat.info(info_flags::nonnegative)) { 72 clog << test_crat 73 << " erroneously recognized as non-negative number" << endl; 74 ++result; 75 } 71 76 72 77 int i = numeric(1984).to_int(); 73 78 if (i-1984) { -
ginac/numeric.cpp
diff --git a/ginac/numeric.cpp b/ginac/numeric.cpp index f05763d..18725b2 100644 (file)
a b bool numeric::info(unsigned inf) const 700 700 case info_flags::negative: 701 701 return is_negative(); 702 702 case info_flags::nonnegative: 703 return !is_negative();703 return is_zero() || is_positive(); 704 704 case info_flags::posint: 705 705 return is_pos_integer(); 706 706 case info_flags::negint:
comment:6 Changed 9 years ago by
Making this change in Pynac does fix it:
sage: abs(pi+i) abs(pi + I) sage: abs(pi+i).n() 3.29690830947562
So now we just need to package this up. See #12501 for the most recently merged Pynac.
Changed 9 years ago by
comment:7 Changed 9 years ago by
Pull request of the patch to the main repository https://bitbucket.org/burcin/pynac/pull-request/1/bugfix-numeric-info-nonnegative-correctly
comment:8 Changed 9 years ago by
- Description modified (diff)
- Reviewers set to Burcin Erocal
Pynac 0.2.4 available in #12950 contains the upstream fix. This ticket can be closed once that is merged.
comment:9 Changed 9 years ago by
- Dependencies set to #12950
- Status changed from new to needs_review
comment:10 Changed 9 years ago by
- Status changed from needs_review to positive_review
comment:11 Changed 9 years ago by
- Merged in set to sage-5.1.beta4
- Resolution set to fixed
- Status changed from positive_review to closed
In the future, be sure to pick a component (for instance, calculus or symbolics); that will help people find it more easily. Thanks for the bug report; mathematically incorrect is definitely bad!
I can confirm this. Moving to major.
Note the following.
Pynac is somehow missing this. Note that it gets e right.