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

Mixing of different domains for symbolic variables #6862

Open
golam-m-hossain opened this issue Sep 2, 2009 · 23 comments
Open

Mixing of different domains for symbolic variables #6862

golam-m-hossain opened this issue Sep 2, 2009 · 23 comments

Comments

@golam-m-hossain
Copy link

From suge-support

On Sep 1, 11:35 pm, Mani chandra mchan...@iitk.ac.in wrote:

Mani chandra wrote:

sage: x = a + I*b
sage: real(x.conjugate().simplify())
real_part(a) + imag_part(b)
sage: real(x.conjugate())
real_part(a) - imag_part(b)

This seems to be happening because maxima(via simplify)
treats variables as real whereas pynac treats as
complex.

sage: x.conjugate()
conjugate(a) - I*conjugate(b)

sage: x.conjugate().simplify()
a - I*b

Upstream: Reported upstream. Developers deny it's a bug.

Component: symbolics

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

@kcrisman
Copy link
Member

kcrisman commented Sep 4, 2009

comment:1

See http://groups.google.com/group/sage-devel/browse_thread/thread/e25e03c9dba88a93

Also, based on the hint there from Robert Dodier, here is the eventual way a fix will have to occur, perhaps as outlined in the thread:

sage: assume(a,'complex')
sage: x.conjugate().simplify()
-I*b + conjugate(a)

@kcrisman
Copy link
Member

comment:2

See also this closely related ask.sagemath.org question, where the following example occurs.

sage: var('a')
a
sage: b=a*a.conjugate()-a*a
sage: b
-a^2 + a*conjugate(a)
sage: simplify(b)
0

I think this is a little weird, though, since in Maxima

(%i1) domain:complex;
(%o1)                               complex
(%i2) -a^2+a*conjugate(a);
(%o2)                                  0

and sadly, the Maxima manual says that all this is expected to do is

Option variable: domain
Default value: real

When domain is set to complex, sqrt (x^2) will remain sqrt (x^2) instead of returning abs(x).

William says in the thread above that

What we need is to queue up (put in some list somewhere) all
declaration that could ever be needed, then whenever we do a Sage -->
calculus Maxima conversion, we would empty the queue if it is
nonempty.  Also, if Maxima were to crash/get restarted (does that ever
happen anymore), we would need to  make sure all var's get set again.
This seems very do-able.

and perhaps that could be part of the initialization process of any variable - without actually calling Maxima at that time, of course!

@kcrisman
Copy link
Member

comment:3

#14628 is somewhat related, though this would not fix it, as far as I can tell.

@kcrisman
Copy link
Member

comment:4

Let's make sure to also test #11656, which was a dup, when (?!) we fix this:

var('c', domain='complex')
var('x', domain='real')
C = c * exp(-x^2)
print (C)
    c*e^(-x^2)

print (C.imag())
    e^(-x^2)*imag_part(c)

print (C.imag().simplify_full()) 
    0

@zimmermann6
Copy link

comment:5

see also #14305

Paul

@kcrisman
Copy link
Member

kcrisman commented Oct 8, 2020

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 13, 2021

comment:7

The result reported in the description is correct :

sage: var("a, b")
(a, b)
sage: c=a+I*b
sage: c.real()
-imag_part(b) + real_part(a)
sage: c.conjugate()
conjugate(a) - I*conjugate(b)
sage: c.conjugate().real()
-imag_part(b) + real_part(a)

//unless a and b are known to be real}}}//. If so :

sage: assume(a, b, "real")
sage: c.real()
a
sage: c.conjugate()
a - I*b
sage: c.conjugate().real()
a

which is also correct.

==> marking as invalid and requesting review in order to get this bug closed...

@kcrisman
Copy link
Member

comment:8

The problem is Maxima, not Sage. (Or rather, the fact that we don't have a good way to make sure that Maxima variables are complex by default, or didn't at the time.)

sage: real(x.conjugate().simplify())
real_part(a) + imag_part(b)

@kcrisman kcrisman added this to the sage-9.4 milestone Mar 13, 2021
@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 13, 2021

comment:9

Replying to @kcrisman:

The problem is Maxima, not Sage. (Or rather, the fact that we don't have a good way to make sure that Maxima variables are complex by default, or didn't at the time.)

sage: real(x.conjugate().simplify())
real_part(a) + imag_part(b)

Unless a and b are known to be real, this is the correct result. When this assumption is verifiable, Sage also gives the expected result (see comment 7)...

BTW, at least the "Computational mathematics with SageMath" book states that SR variables behave, by default, as complex variables, but, IIRC, no formal assertion is made in the documentation about this. AFAICT, we use a"domain:complex" assertion in our uses of Maxima.

So what should be the behavior you expect ? OIn fact, I'm having troubleperceivng the point of this ticket...

==> re-asking review ; possibly after discussion on sage-devel if we can't agree...

@EmmanuelCharpentier EmmanuelCharpentier mannequin removed this from the sage-9.4 milestone Mar 13, 2021
@kcrisman
Copy link
Member

comment:10
sage: real(x.conjugate().simplify())
real_part(a) + imag_part(b)

I thought you said this one was correct:

sage: c.conjugate().real()
-imag_part(b) + real_part(a)

So you can see that the Maxima (.simplify()) and Sage result are different, unless I'm even more confused.

BTW, at least the "Computational mathematics with SageMath" book states that SR variables behave, by default, as complex variables, but, IIRC, no formal assertion is made in the documentation about this. AFAICT, we use a"domain:complex" assertion in our uses of Maxima.

The problem is that domain:complex doesn't make the Maxima variables complex, it just doesn't simplify square roots (see the linked sage-devel thread in comment:1). We do use complex variables for SR but those live in Pynac. However, since .simplify() is a Sage method (it just sends to Maxima and back), we don't want that giving wrong behavior.

Anyway, perhaps we need a third party to adjudicate; I did try to suss out the right behavior after your comment:7 but I have been known to make sign errors in my life :-) But I hope I clarified the exact status in this comment.

var("a, b")
c=a+I*b
print(c.conjugate().real())
print((c.conjugate().simplify()).real())

gives

-imag_part(b) + real_part(a)
imag_part(b) + real_part(a)

and I don't think those can both be correct.

@nbruin
Copy link
Contributor

nbruin commented Mar 14, 2021

comment:11

Replying to @kcrisman:

Anyway, perhaps we need a third party to adjudicate; I did try to suss out the right behavior after your comment:7 but I have been known to make sign errors in my life :-) But I hope I clarified the exact status in this comment.

var("a, b")
c=a+I*b
print(c.conjugate().real())
print((c.conjugate().simplify()).real())

gives

-imag_part(b) + real_part(a)
imag_part(b) + real_part(a)

and I don't think those can both be correct.

Indeed; I'd say that the problem diagnosed in the ticket is spot on. I also don't know what the best solution is. Note that the two results are both correct if imag_part(b)==0, which is what maxima assumes (and we inherit those assumptions by the implementation of simplify).

A minimal solution would be to document (in simplify and/or in real_part, imag_part, and conjugate) that for simplify, symbolic variables are assumed to be real, so that conjugate(x).simplify()==real_part(x).simplify()==x and imag_part(x).simplify()==0.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 14, 2021

comment:12

Replying to @nbruin:

[ Snip... ]

Indeed; I'd say that the problem diagnosed in the ticket is spot on.

Indeed...

I also don't know what the best solution is. Note that the two results are both correct if imag_part(b)==0, which is what maxima assumes (and we inherit those assumptions by the implementation of simplify).

A minimal solution would be to document (in simplify and/or in real_part, imag_part, and conjugate) that for simplify, symbolic variables are assumed to be real, so that conjugate(x).simplify()==real_part(x).simplify()==x and imag_part(x).simplify()==0.

At the (accepted) risk of lampooning the British Commons : No, no, no, no, no, no, no, no.

And no...

This assumption is a way too large limitation of Sage's algebraic abilities. And can't be enforced by checks...

Finding the source of the problem is necessary. It might help to show that the problem is alleviated by renaming the variables :

sage: %cpaste
Pasting code; enter '--' alone on the line to stop or use Ctrl-D.
:var('a,b')
:c=a+I*b
:L=c.operands()
:aL=[maxima.gensym().sage() for u in L]
:D=dict(zip(L,aL))
:DI=dict(zip(aL,L))
:print(c.conjugate().real())
:print(c.conjugate().simplify().real())
:print(c.subs(D).conjugate().real().subs(DI))
:print(c.subs(D).conjugate().simplify().real().subs(DI))
:--
(a, b)
-imag_part(b) + real_part(a)
imag_part(b) + real_part(a)
-imag_part(b) + real_part(a)
-imag_part(b) + real_part(a)

However :

sage: print(c.conjugate().subs(D).simplify().real().subs(DI))
imag_part(b) + real_part(a)

This behaviour let us think that somehow, simplify doesn't account for the "complexity"of (-I*b"). Another hint in this direction :

sage: with assuming(a,b,"real"):c.conjugate().simplify().real()
a
sage: with assuming(a,b,"complex"):c.conjugate().simplify().real()
-imag_part(b) + real_part(a)

which is correct.

This suggests a direction for debugging the source (which is probably in pynac territory, i. e. put of my reach...) and a possible workaround : bracket calls to Maxima's simplify with an explicit assumption of complexity for all variables not declared otherwise... This is problematic, however, since simplify just converts its argument to Maxima and back. Following the relevant code isn't exactly easy...

However, the real problem is probably not in Maxima itself :

(%i1) display2d:false;

(%o1) false
(%i2) domain:complex;

(%o2) complex
(%i3) c:a+%i*b;

(%o3) %i*b+a
(%i4) realpart(conjugate(c));

(%o4) a
(%i5) realpart(ratsimp(conjugate(c)));

(%o5) a

Notwithstanding the domain setting, Maxima acts as if a and b were real.

(%i6) declare(a, complex, b, complex);

(%o6) done
(%i7) realpart(conjugate(c));

(%o7) 'realpart(a)-'imagpart(b)
(%i8) realpart(ratsimp(conjugate(c)));

(%o8) 'realpart(a)-'imagpart(b)

Maxima's ratsimp does not create the same problem as Sages simplify`.

HTH,

@nbruin
Copy link
Contributor

nbruin commented Mar 14, 2021

comment:13

Replying to @EmmanuelCharpentier:

This behaviour let us think that somehow, simplify doesn't account for the "complexity"of (-I*b"). Another hint in this direction :

sage: with assuming(a,b,"real"):c.conjugate().simplify().real()
a
sage: with assuming(a,b,"complex"):c.conjugate().simplify().real()
-imag_part(b) + real_part(a)

which is correct.

Nice find! That is consistent with "maxima by default assumes variables are real-valued as far as conjugate, real_part, imag_part as concerned", so there's a mismatch between sage/pynac and maxima what the default assumptions about variables is, and indeed the appropriate work-around is to sync up those assumptions to match (one way or the other).

I'm not so sure if we should bracket each simplify call with an assume. I think it's the responsibility of the interface to translate x into a symbol in the other system with the right properties. So I think that maxima_calculus(x) should basically already insert the assumption on the maxima side, unless x is assumed to be real: mismatching defaults on the maxima side just mean we cannot rely on the default behaviour there and we need to enforce the desired behaviour appropriately.

I'd also be fine with changing the assumption that variables are by default real unless declared otherwise. If we want to change our default assumption about variables, we may need to change pynac. Otherwise, I think it's a matter of changing the maxima interface (mainly the maxima_calculus one).

@kcrisman
Copy link
Member

comment:14

Nice find! That is consistent with "maxima by default assumes variables are real-valued as far as conjugate, real_part, imag_part as concerned",

Correct. As I mention above, domain:complex is useful but doesn't affect much beyond sqrt. And I doubt Maxima will be changing that.

I'm not so sure if we should bracket each simplify call with an assume. I think it's the responsibility of the interface to translate x into a symbol in the other system with the right properties. So I think that maxima_calculus(x) should basically already insert the assumption on the maxima side, unless x is assumed to be real: mismatching defaults on the maxima side just mean we cannot rely on the default behaviour there and we need to enforce the desired behaviour appropriately.

Yes, we should be sending the correct thing to Maxima. The problem is that it might be hard to parse out every symbol and make sure it has all the right extra assumptions, or at least in the past that seems to have led into a rat's nest. We do prepend sage_var or something like that to each Sage variable in Maxima, so at least in theory it should be possible, but one wouldn't want to overwrite previous assumptions, so a lot of testing would be involved. It would be really nice, of course!

I'd also be fine with changing the assumption that variables are by default real unless declared otherwise. If we want to change our default assumption about variables, we may need to change pynac.

I think that changing all variables to real by default probably would be a bad move in many ways. (I don't think you're suggesting that, but the way you phrased it sounds like that.)

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 15, 2021

comment:15

The Maxima problem has been reported upstream.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 15, 2021

Upstream: Reported upstream. No feedback yet.

@EmmanuelCharpentier EmmanuelCharpentier mannequin added this to the sage-9.3 milestone Mar 15, 2021
@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 15, 2021

comment:16

Priority set to critical because this bug silenty leads to mathematically incorrect results.

@nbruin
Copy link
Contributor

nbruin commented Mar 15, 2021

comment:17

Replying to @kcrisman:

Yes, we should be sending the correct thing to Maxima. The problem is that it might be hard to parse out every symbol and make sure it has all the right extra assumptions, or at least in the past that seems to have led into a rat's nest. We do prepend sage_var or something like that to each Sage variable in Maxima, so at least in theory it should be possible, but one wouldn't want to overwrite previous assumptions, so a lot of testing would be involved. It would be really nice, of course!

That should actually be dead-easy. This is not about sending strings over that need to be parsed for variables; this is about sending expressions over. They are already parsed. Especially if you do that in the way that maxima_lib works, the symbol needs to be created on the maxima side. If we assume our variables to be complex by default, then that assumption should be inserted at that time. If assumptions change, we just need to do whatever dance we do already to change them on the maxima side as well.

The main problem I expect is that inserting the assumption will probably lead to other side-effects we didn't anticipate. That's why I figured documenting the current "simplify" behaviour is the easier way out (but I wouldn't trust non-trivial SR computations for publication-quality work anyway).

@kcrisman
Copy link
Member

comment:18

That should actually be dead-easy. This is not about sending strings over that need to be parsed for variables; this is about sending expressions over. They are already parsed. Especially if you do that in the way that maxima_lib works, the symbol needs to be created on the maxima side. If we assume our variables to be complex by default, then that assumption should be inserted at that time. If assumptions change, we just need to do whatever dance we do already to change them on the maxima side as well.

Good. And I certainly only care about the maxima_lib case.

The main problem I expect is that inserting the assumption will probably lead to other side-effects we didn't anticipate. That's why I figured documenting the current "simplify" behaviour is the easier way out (but I wouldn't trust non-trivial SR computations for publication-quality work anyway).

True.

The Maxima problem has been ​reported upstream.

I expect they will say this is user error or won't implement, since the documentation makes it pretty clear that domain:complex doesn't do much, and presumably (though by implication only) shouldn't be expected to do much else.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 21, 2021

comment:19

According to Stavros Macrakis,domain is supposed to have an effect only for power operations (and is not expected to have an effect on realpart). He reclassified the issue as a documentation issue (wrongly, IMHO, but nothing can be done...).

We should therefore fox it at Sage's level...

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 21, 2021

Changed upstream from Reported upstream. No feedback yet. to Reported upstream. Developers deny it's a bug.

@DaveWitteMorris
Copy link
Member

comment:20

Related ticket: #30793.

@mkoeppe
Copy link
Contributor

mkoeppe commented May 10, 2021

comment:21

Moving to 9.4, as 9.3 has been released.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 May 10, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 9, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 7, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
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

6 participants