Opened 9 years ago
Closed 8 years ago
#11155 closed defect (fixed)
abs(pi+I) = pi+I
Reported by:  benreynwar  Owned by:  burcin 

Priority:  critical  Milestone:  sage5.1 
Component:  symbolics  Keywords:  
Cc:  burcin  Merged in:  sage5.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 9 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 8 years ago by
 Cc burcin added
 Description modified (diff)
 Milestone set to sage5.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 8 years ago by
Reported to the ginacdevel 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 8 years ago by
Apparently already fixed!
comment:5 Changed 8 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 nonnegative number" << endl; 74 ++result; 75 } 71 76 72 77 int i = numeric(1984).to_int(); 73 78 if (i1984) { 
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 8 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 8 years ago by
comment:7 Changed 8 years ago by
Pull request of the patch to the main repository https://bitbucket.org/burcin/pynac/pullrequest/1/bugfixnumericinfononnegativecorrectly
comment:8 Changed 8 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 8 years ago by
 Dependencies set to #12950
 Status changed from new to needs_review
comment:10 Changed 8 years ago by
 Status changed from needs_review to positive_review
comment:11 Changed 8 years ago by
 Merged in set to sage5.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.