is_polynomial returns wrong results

[hivert]

sage: el = -1/2*(2*x^2 - sqrt(2*x - 1)*sqrt(2*x + 1) - 1)
sage: el.is_polynomial(x)
True

I definitely prefer to get False.

Apply attachment: trac_11352-is_polynomial.patch.

Depends on #11415

Upstream: Fixed upstream, in a later stable release.

CC: @kcrisman @burcin @hivert @RBKreckel

Component: symbolics

Keywords: is_polynomial

Author: Richard Kreckel, Jens Vollinga, Burcin Erocal

Reviewer: Karl-Dieter Crisman

Merged: sage-4.7.1.alpha4

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

[vbraun]

comment:1

We just return GiNaC's response:

  cdef Expression symbol0 = self.coerce_in(var)
  return self._gobj.is_polynomial(symbol0._gobj)

[hivert]

comment:2

Replying to @vbraun:

We just return GiNaC's response:

  cdef Expression symbol0 = self.coerce_in(var)
  return self._gobj.is_polynomial(symbol0._gobj)

So this probably must be reported to GiNaC, isn't it ?

[hivert]

comment:5

I just installed a fresh GiNaC on my computer and I've been able to reproduce the bug:

#include <iostream>
#include <ginac/ginac.h>

using namespace std;
using namespace GiNaC;

int main()
{
  symbol x("x");
  ex poly;

  poly = sqrt(x*x+1)*sqrt(x+1);

  cout << poly << endl;
  cout << poly.is_polynomial(x) << endl;
  return 0;
}

gives

sqrt(1+x^2)*sqrt(1+x)
1

So I subscribed to ginac mailing list and reported the bug here. I'm waiting for an answer.

Florent

[hivert]

Upstream: Reported upstream. Little or no feedback.

[kcrisman]

comment:6

So I subscribed to ginac mailing list and reported the bug here. I'm waiting for an answer.

Thanks for doing that, Florent. By the way, can you put a link to the thread on the Ginac list here?

[hivert]

comment:7

Thanks for doing that, Florent. By the way, can you put a link to the thread on the Ginac list here?

Sure ! Here it is:

http://www.ginac.de/pipermail/ginac-list/2011-May/001822.html

[hivert]

comment:8

I just got the following answer:

thanks for reporting the bug. I am about to fix it.

I'm not sure what will happen next: Should we backport the fix or update the whole ginac ?

[hivert]

Changed upstream from Reported upstream. Little or no feedback. to Reported upstream. Developers acknowledge bug.

[hivert]

Work Issues: wait for upstream fix

[hivert]

Changed work issues from wait for upstream fix to none

[hivert]

comment:9

The bug seems to be fixed upstream, thanks to Richard B. Kreckel:

Anyway, in this case you'll have to port these two patches to Pynac:

http://www.ginac.de/ginac.git?p=ginac.git;a=commitdiff;h=0c2f0f4c6d

http://www.ginac.de/ginac.git?p=ginac.git;a=commitdiff;h=293ff6f6fe

As I previously said, I'm not sure what to do next. Moreover, I'd rather let who knows about PyNaC and Sage symbolic internal handle the rests of this ticket. Please feel free to take ownership of it.

[hivert]

Changed upstream from Reported upstream. Developers acknowledge bug. to Fixed upstream, in a later stable release.

[burcin]

comment:10

Thanks for following this through so far Florent. The next step is for me to make a new Pynac release with these patches. I will do this in the next few days, but it might be a while before this lands in a Sage release, since the next version of pynac will include fixes for #9880 and an upstream patch for #10964.

[hivert]

comment:11

Hi Burcin,

Replying to @burcin:

Thanks for following this through so far Florent. The next step is for me to make a new Pynac release with these patches. I will do this in the next few days, but it might be a while before this lands in a Sage release, since the next version of pynac will include fixes for #9880 and an upstream patch for #10964.

I'm keeping an eye here. Count me as a volunteer if some help is needed for the review.

[hivert]

comment:13

Attachment: trac_11352-is_polynomial.patch.gz

From the Ginac mailing list:

Hi everybody,

GiNaC 1.6.0 is out and available. You can find the changes at the end
of this email.

Note:
This release is not binary compatible to the previous one.
Also, we changed the name of the library filename from libginac-1.5 to
libginac. The ABI versioning is now done in recommended libtool
manner. The dynamic library will be created as libginac.so.2.0.0 this
time.
We also stopped to create extra branches in the repository for ABI
compatible versions. The latest releases will from now on always be in
master.

Changes:
[...]
* Fixed a bug in is_polynomial().
[...]

So we now "only" need to upgrade ginac.

There is no "fixed upstream and released" tag... I'm not sure if the current tag is correct.
Florent

[burcin]

comment:14

I won't rant about the tags. :) Note that this is still needs_info.

Upstream is pynac in this case. I ported the patches in the GiNaC release to Pynac (this is easier than it sounds, you just ignore changes to files that don't exist), but didn't make a release yet. Here is the queue of patches which will be merged:

https://bitbucket.org/burcin/pynac-patches/src

[burcin]

Dependencies: #11415

[burcin]

comment:15

New pynac package which includes the patch for this is at #11415.

attachment: trac_11352-is_polynomial.patch adds doctests.

[burcin]

Author: Richard Kreckel, Jens Vollinga, Burcin Erocal

[kcrisman]

comment:16

Positive review.

[kcrisman]

Reviewer: Karl-Dieter Crisman

[kcrisman]

comment:19

Oops, browser windows closed too fast - I was on #10964 still :)

[kcrisman]

comment:20

Wiki markup here is different than on Sage wiki :(

[kcrisman]

comment:21

Okay, now positive review! Good catch by rbk, incidentally, with the sqrt(2)*x issue.

[jdemeyer]

Merged: sage-4.7.1.alpha4