Opened 13 years ago
Closed 11 years ago
#8497 closed defect (fixed)
bug in simplify_radical
Reported by: | Paul Zimmermann | Owned by: | Burcin Erocal |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7.2 |
Component: | calculus | Keywords: | simplify, radical, sqrt |
Cc: | Karl-Dieter Crisman, Burcin Erocal, Jason Grout, Mike Hansen | Merged in: | sage-4.7.2.alpha4 |
Authors: | Paul Zimmermann, Jeroen Demeyer | Reviewers: | Burcin Erocal |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
the documentation of simplify_radical
says:
sage: x.simplify_radical? ... Simplifies this symbolic expression, which can contain logs, exponentials, and radicals, by converting it into a form which is canonical over a large class of expressions and a given ordering of variables
however if indeed it is able to recognize zero:
sage: a=1/(sqrt(5)+sqrt(2))-(sqrt(5)-sqrt(2))/3 sage: a.simplify_radical() 0
it does *not* return a canonical expression:
sage: a1=1/(sqrt(5)+sqrt(2)) sage: a2=(sqrt(5)-sqrt(2))/3 sage: a1.simplify_radical() 1/(sqrt(2) + sqrt(5)) sage: a2.simplify_radical() -1/3*sqrt(2) + 1/3*sqrt(5) sage: (a1-a2).simplify_radical() 0
Apply only 8497_fix_doc.patch
Attachments (2)
Change History (24)
comment:1 Changed 13 years ago by
comment:2 Changed 13 years ago by
Summary: | bug in simplify_rational → bug in simplify_radical |
---|
This is the full docstring from Maxima:
Simplifies expr, which can contain logs, exponentials, and radicals, by converting it into a form which is canonical over a large class of expressions and a given ordering of variables; that is, all functionally equivalent forms are mapped into a unique form. For a somewhat larger class of expressions, radcan produces a regular form. Two equivalent expressions in this class do not necessarily have the same appearance, but their difference can be simplified by radcan to zero. For some expressions radcan is quite time consuming. This is the cost of exploring certain relationships among the components of the expression for simplifications based on factoring and partial-fraction expansions of exponents.
Perhaps we should include this
comment:3 Changed 13 years ago by
Perhaps we should include this
yes (unless of course upstream finds a way to get a real canonical form). And maybe adding an example showing the difference when checking for 0.
comment:4 Changed 12 years ago by
Cc: | Karl-Dieter Crisman added |
---|
comment:5 Changed 11 years ago by
What is really going on here is that simplify_radical
uses radcan
under the hood, which treats things as symbolic expressions, not functions, and just chooses a branch - consistently, but arbitrarily. At least I think that is what is going on here. See the Maxima list thread starting here.
comment:6 Changed 11 years ago by
then should we simply change the documentation, in saying that simplify_radical
gives a
canonical expression for zero, but if a and b are two identical expressions, they might not
be "simplified" to the same expression?
Paul
comment:7 Changed 11 years ago by
You are correct. I was just updating, though, at this point.
It gets worse, because some expressions that are definitely not 1 will simplify to just 1, because that is the branch that was picked. See this ask.sagemath.org question, and Fateman's accurate response.
comment:8 Changed 11 years ago by
then I suggest to simply remove this function from Sage, unless there are ideas how to fix it.
Paul
comment:9 Changed 11 years ago by
Cc: | Burcin Erocal Jason Grout Mike Hansen added |
---|
Well, in Fateman's eyes (and I would remind that he is an expert, if not THE expert, in this), the only bug is in users who treat these expressions as functions. At least, that's how I interpret it. So updating the documentation may be the better way.
But this shouldn't be a duologue; hopefully some others will have ideas. Cc:ing a few others who have thought about at least one or two of these things, just in case they have thoughts at this time. Otherwise it will sit - I simply don't have time to deal with it right now, because it needs to be part of a general overhaul of simplification if we don't just change documentation.
comment:10 Changed 11 years ago by
I mean change documentation to give examples in prominent places, both in simplify_radical
and simplify_full
.
comment:11 Changed 11 years ago by
I believe we should at least add such examples to the documentation, to warn the user that in some cases no canonical form is returned.
Paul
comment:12 Changed 11 years ago by
Okay. So whoever does this ticket will do that :)
(Incidentally, mentioning that they are canonical, but in Fateman's sense of expressions, not in the way we would think of them as functions.)
Changed 11 years ago by
Attachment: | trac_8497.patch added |
---|
comment:13 Changed 11 years ago by
Status: | new → needs_review |
---|
the attached patch implements what I suggest in comment 11.
Paul
comment:14 Changed 11 years ago by
Authors: | → Paul Zimmermann |
---|---|
Reviewers: | → Burcin Erocal |
Status: | needs_review → positive_review |
Looks good to me.
comment:15 Changed 11 years ago by
Authors: | Paul Zimmermann → Paul Zimmermann, Jeroen Demeyer |
---|---|
Description: | modified (diff) |
Status: | positive_review → needs_work |
Fixed some formatting of the documentation, needs review.
comment:16 Changed 11 years ago by
Status: | needs_work → needs_review |
---|
comment:17 Changed 11 years ago by
Status: | needs_review → needs_info |
---|
I feel that we should at least ask on the Maxima list about whether this is truly "not canonical". My understanding is that Fateman would say it is canonical as an expression, not as a function. If I'm the only one who feels this way, I'll let it slide. But I figure we would want him to give us benefit of the doubt in our areas of expertise.
comment:18 follow-up: 19 Changed 11 years ago by
For me, a "canonical expression" means that two mathematically identical expressions simplify to the *exactly same* expression. Thus it is clearly not canonical. Feel free to ask on the Maxima list, but this should not delay this ticket.
Paul
comment:19 Changed 11 years ago by
Status: | needs_info → needs_review |
---|
Replying to zimmerma:
this should not delay this ticket.
I agree but somebody needs to review my reformatting of the documentation.
comment:20 Changed 11 years ago by
Status: | needs_review → positive_review |
---|
I am not well versed in ReST, but AFAICT, Jeroen's changes make sense.
Maxima documentation on radcan()
(below) is rather vague. Based on this text, we shouldn't make bold claims about canonical results in the Sage documentation. I am switching this back to positive review.
Simplifies expr, which can contain logs, exponentials, and radicals, by converting it into a form which is canonical over a large class of expressions and a given ordering of variables; that is, all functionally equivalent forms are mapped into a unique form. For a somewhat larger class of expressions, radcan produces a regular form. Two equivalent expressions in this class do not necessarily have the same appearance, but their difference can be simplified by radcan to zero. For some expressions radcan is quite time consuming. This is the cost of exploring certain relationships among the components of the expression for simplifications based on factoring and partial-fraction expansions of exponents.
We can open an enhancement ticket to clarify what
- a large class of expressions
- functionally equivalent
- regular form
mean in the text above, and how the ordering of the variables effect the final result. Ideally, we should have references to a description of the underlying algorithm as well.
comment:22 Changed 11 years ago by
Merged in: | → sage-4.7.2.alpha4 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Note the original question posed to me was: how to multiply say 1/(1+sqrt(2)+sqrt(3)) by the conjugate expression?