Opened 10 years ago

Closed 9 years ago

# is_polynomial returns wrong results

Reported by: Owned by: hivert burcin major sage-4.7.1 symbolics is_polynomial kcrisman, burcin, hivert, rbk sage-4.7.1.alpha4 Richard Kreckel, Jens Vollinga, Burcin Erocal Karl-Dieter Crisman Fixed upstream, in a later stable release. #11415

```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`.

### comment:1 follow-up: ↓ 2 Changed 10 years ago by vbraun

We just return GiNaC's response:

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

### comment:2 in reply to: ↑ 1 Changed 10 years ago by hivert

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 ?

### comment:3 Changed 10 years ago by hivert

• Description modified (diff)
• Owner changed from burcin to hivert

### comment:5 follow-up: ↓ 6 Changed 10 years ago by hivert

• Report Upstream changed from N/A to Reported upstream. Little or no feedback.

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

### comment:6 in reply to: ↑ 5 ; follow-up: ↓ 7 Changed 10 years ago by kcrisman

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?

### comment:7 in reply to: ↑ 6 Changed 10 years ago by hivert

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:

### comment:8 Changed 10 years ago by hivert

• Report Upstream changed from Reported upstream. Little or no feedback. to Reported upstream. Developers acknowledge bug.
• Status changed from new to needs_info
• Work issues set to wait for upstream fix

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 ?

### comment:9 Changed 10 years ago by hivert

• Report Upstream changed from Reported upstream. Developers acknowledge bug. to Fixed upstream, in a later stable release.
• Work issues wait for upstream fix deleted

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:

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.

### comment:10 follow-up: ↓ 11 Changed 10 years ago by burcin

• Owner changed from hivert 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.

### comment:11 in reply to: ↑ 10 Changed 10 years ago by hivert

Hi 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.

### comment:13 Changed 10 years ago by hivert

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

### comment:14 Changed 10 years ago by burcin

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:

### comment:15 Changed 10 years ago by burcin

• Authors set to Richard Kreckel, Jens Vollinga, Burcin Erocal
• Dependencies set to #11415
• Description modified (diff)
• Milestone changed from sage-4.7.1 to sage-5.0
• Status changed from needs_info to needs_review

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

### comment:16 Changed 9 years ago by kcrisman

• Reviewers set to Karl-Dieter Crisman
• Status changed from needs_review to positive_review

Positive review.

### comment:17 Changed 9 years ago by kcrisman

• Description modified (diff)

### comment:18 Changed 9 years ago by kcrisman

• Status changed from positive_review to needs_work

### comment:19 Changed 9 years ago by kcrisman

• Status changed from needs_work to needs_review

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

### comment:20 Changed 9 years ago by kcrisman

• Description modified (diff)

Wiki markup here is different than on Sage wiki :(

### comment:21 Changed 9 years ago by kcrisman

• Status changed from needs_review to positive_review

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

### comment:22 Changed 9 years ago by jdemeyer

• Milestone changed from sage-5.0 to sage-4.7.1

### comment:23 Changed 9 years ago by jdemeyer

• Merged in set to sage-4.7.1.alpha4
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.