expairseq::conjugate
Hi, the following problem (in Sage syntax) was reported to the Sage issue tracker [1]: ginsh - GiNaC Interactive Shell (ginac V1.5.8) <snip>
conjugate(I*sqrt(-3)); -I*sqrt(-3)
[1] http://trac.sagemath.org/sage_trac/ticket/10964 The problem seems to be the expair::conjugate() method which is called from the conjugateepvector() function in expairseq.cpp. This function is called from the pseries class as well. So, moving the latter in the expairseq class to use the virtual functions recombine_pair_to_ex() etc. does not work. Any ideas for a clean solution to this? Thank you. Burcin
Hi Burcin, On 05/08/2011 09:42 PM, Burcin Erocal wrote:
the following problem (in Sage syntax) was reported to the Sage issue tracker [1]:
ginsh - GiNaC Interactive Shell (ginac V1.5.8) <snip>
conjugate(I*sqrt(-3)); -I*sqrt(-3)
Indeed. The patch I committed a year ago to make the branch cut tranformations less aggressive forgets about complex conjugation of mul objects containing roots. Of course, the imaginary I in the example above is not relevant. Here is another way (and a good regression test, BTW) to see it fail. In ginsh:
subs(conjugate(a*sqrt(-2))-a*conjugate(sqrt(-2)), a==1); -conjugate(sqrt(-2))+sqrt(-2)
[1] http://trac.sagemath.org/sage_trac/ticket/10964
The problem seems to be the expair::conjugate() method which is called from the conjugateepvector() function in expairseq.cpp. This function is called from the pseries class as well. So, moving the latter in the expairseq class to use the virtual functions recombine_pair_to_ex() etc. does not work.
Any ideas for a clean solution to this?
Ah, I see that you already got very close to fixing it. Well, what remains to be done is to ensure that the code dispatches to the right recombine_pair_to_ex()/split_ex_to_pair() methods. For this, it needs the vptr of the expairseq object. It doesn't have to be a member function, though: We can pass it as an extra argument to conjugateepvector(), in order to retain binary compatibility. I'ld propose to fix it similar to the patch attached. It passes GiNaC's regression tests and works as expected:
subs(conjugate(a*sqrt(-2))-a*conjugate(sqrt(-2)), a==1); 0
What do you think? I'll clean it up a bit and push it to the GiNaC repository later, when I'm less sleepy. (:| -richy. -- Richard B. Kreckel <http://www.ginac.de/~kreckel/>
Hi Alexei and Jens, On 05/11/2011 12:13 AM, Richard B. Kreckel wrote:
[...] We can pass it as an extra argument to conjugateepvector(), in order to retain binary compatibility.
I'ld propose to fix it similar to the patch attached. [...]
Without the maintenance branch, what now? That patch goes to some lenghts in order to retain binary compatibility. Should I bother or should I simplify it and bump the soname version? -richy. -- Richard B. Kreckel <http://www.ginac.de/~kreckel/>
Hi, On 11.05.2011 00:17, Richard B. Kreckel wrote:
Hi Alexei and Jens,
On 05/11/2011 12:13 AM, Richard B. Kreckel wrote:
[...] We can pass it as an extra argument to conjugateepvector(), in order to retain binary compatibility.
I'ld propose to fix it similar to the patch attached. [...]
Without the maintenance branch, what now?
That patch goes to some lenghts in order to retain binary compatibility. Should I bother or should I simplify it and bump the soname version?
don`t bother with binary compatibility!! Next release will be 1.6.0 and by convention people will (or have to) expect that if the minor number changes there might be an ABI breakage. And I'll make it a habit to announce ABI breaks in the release announcement, so no surprises. Regards Jens
Hi! On 05/11/2011 12:24 AM, Jens Vollinga wrote:
don`t bother with binary compatibility!!
I came to the conclusion that the best way of fixing the bug is by overriding the virtual function conjugate() in class mul. This also retains binary compatibility.
Next release will be 1.6.0 and by convention people will (or have to) expect that if the minor number changes there might be an ABI breakage.
And I'll make it a habit to announce ABI breaks in the release announcement, so no surprises.
What is the reason for a new ABI version? I just checked and found that current HEAD is still compatible with GiNaC 1.5.0. Putting on my distributor's hat, I hate the trouble caused by new SONAME versions. -richy. -- Richard B. Kreckel <http://www.ginac.de/~kreckel/>
Hi, On 15.05.2011 22:01, Richard B. Kreckel wrote:
Next release will be 1.6.0 and by convention people will (or have to) expect that if the minor number changes there might be an ABI breakage.
And I'll make it a habit to announce ABI breaks in the release announcement, so no surprises.
What is the reason for a new ABI version? I just checked and found that current HEAD is still compatible with GiNaC 1.5.0. Putting on my distributor's hat, I hate the trouble caused by new SONAME versions.
you misunderstood me. I won't raise the soname, but I call the release 1.6.0. So it is a new minor number and that shall mean for users/distros: keep an eye on the release announcement, because the new release just _might_ have a new soname. If there is no information in the release announcement, then the ABI didn't change. I just don't want to call the release 1.5.9, mainly because of the new branching policy in the git repo. And also in the future it might be a good idea to signal important changes in the minor number (if not the major number). Regards, Jens BTW, I am just looking at PG CLARK's bug. Don't know how fast I can move, yet.
participants (3)
-
Burcin Erocal
-
Jens Vollinga
-
Richard B. Kreckel