Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

abs(a+b)^2 == (a+b)^2 #11668

Closed
sagetrac-jan mannequin opened this issue Aug 8, 2011 · 8 comments
Closed

abs(a+b)^2 == (a+b)^2 #11668

sagetrac-jan mannequin opened this issue Aug 8, 2011 · 8 comments

Comments

@sagetrac-jan
Copy link
Mannequin

sagetrac-jan mannequin commented Aug 8, 2011

sage: version()
'Sage Version 4.7, Release Date: 2011-05-23'
sage: var('a b', domain='real');
sage: A = abs((a+i*b))^2
sage: A
abs(a + I*b)^2
sage: imag(A)
0
sage: A.simplify_full()
a^2 + 2*I*a*b - b^2
sage: imag(A.simplify_full())
2*a*b

The last result is clearly wrong. abs() is real by definition. simplify_full() doesn't handle abs() correctly.

Demo Notebook: http://demo.sagenb.org/home/pub/181/

CC: @kcrisman @orlitzky

Component: symbolics

Keywords: maxima abs

Author: Karl-Dieter Crisman

Reviewer: Michael Orlitzky

Merged: sage-5.11.beta3

Issue created by migration from https://trac.sagemath.org/ticket/11668

@sagetrac-jan sagetrac-jan mannequin added this to the sage-5.11 milestone Aug 8, 2011
@sagetrac-jan sagetrac-jan mannequin assigned ncalexan and burcin and unassigned ncalexan Aug 8, 2011
@kcrisman
Copy link
Member

kcrisman commented Aug 9, 2011

comment:2

The exact place is

sage: A.simplify_radical()
a^2 + 2*I*a*b - b^2

This is how radcan in Maxima does this over the real domain, which simplify_radical temporarily makes us in.

I'm not sure what the best thing to do here is. Two ideas:

  • Change so that we stay in domain:complex in Maxima, sort of missing the point of simplify_radical
  • Give lots of warnings not to trust simplification because (like most simplification) it is not always true, but useful. (Sort of like how \sqrt{x^2}\neq x but sometimes it's useful to simplify it that way.)

I do not think we pass the domain information from the (Pynac) variables in any case, or if that would be very easy to do.

@kcrisman
Copy link
Member

comment:3

This has been fixed by #12780.

sage: var('a b')
(a, b)
sage: A = abs((a+i*b))^2
sage: A.simplify_radical()
abs(a + I*b)^2
sage: imag(A)
0
sage: imag(A.simplify_full())
0

Patch probably coming up. This would also be dealt with by #12737, but that is orthogonal to having fixed the domain issues.

@kcrisman
Copy link
Member

Author: Karl-Dieter Crisman

@orlitzky
Copy link
Contributor

comment:5

Two minor things, can you use a,b = SR.var('a,b') to create the variables? That will eliminate one line of unrelated output. And just a style preference, since sage outputs the complex unit as I, I think it's a tiny bit more consistent to use that for input as well, but I'll take it either way.

@kcrisman
Copy link
Member

Attachment: trac_11668.patch.gz

Based on 5.10.rc1

@kcrisman
Copy link
Member

comment:6

Two minor things

Sure, no problem. I put the i instead of I just because I copied from the original example in the ticket, but that is unimportant. Thanks!

@orlitzky
Copy link
Contributor

Reviewer: Michael Orlitzky

@jdemeyer
Copy link

Merged: sage-5.11.beta3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants