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

bug in collect and/or term ordering in symbolics #9046

Closed
zimmermann6 opened this issue May 25, 2010 · 18 comments
Closed

bug in collect and/or term ordering in symbolics #9046

zimmermann6 opened this issue May 25, 2010 · 18 comments

Comments

@zimmermann6
Copy link

This seems a bug (note the instances of -x^2 and x^2):

var('a b x y z')
sage: p = -a*x^3 - a*x*y^2 + 2*b*x^2*y + 2*y^3 + x^2*z + y^2*z + x^2 + y^2 + a*x
sage: p.collect(x)
-a*x^3 + (2*b*y + z + 1)*x^2 - x^2 - (a*y^2 - a)*x + x^2 + 2*y^3 + y^2*z + y^2

Depends on #9880

CC: @kcrisman

Component: symbolics

Keywords: pynac

Reviewer: Burcin Erocal, Paul Zimmermann

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

@burcin
Copy link

burcin commented May 26, 2010

comment:1

Here is the same session using GiNaC directly via ginsh:

> t= -a*x^3 - a*x*y^2 + 2*b*x^2*y + 2*y^3 + x^2*z + y^2*z + x^2 + y^2 + a*x;
x^2+2*y*b*x^2+y^2+y^2*z+a*x+2*y^3-a*x^3-y^2*a*x+z*x^2
> t;
x^2+2*y*b*x^2+y^2+y^2*z+a*x+2*y^3-a*x^3-y^2*a*x+z*x^2
> collect(t, x);
(1+2*y*b+z)*x^2+y^2+y^2*z+2*y^3-a*x^3-(y^2*a-a)*x
> u = (x^2+(y-x^2)*(y+x));
x^2-(y+x)*(x^2-y)
> collect(u, x);
x^3-(x^2-y)*x+y^2-(-1+y+x)*x^2

It seems that one needs to call expand() explicitly before calling collect(). I think this should just be documented in the docstring.

The problem with -x^2 + x^2 appearing in the output is probably a bug I introduced while playing with the ordering of the terms. I will take a look at it when I find a chance. It's likely to be later than a week though.

@burcin
Copy link

burcin commented May 26, 2010

Changed keywords from none to pynac

@kcrisman
Copy link
Member

comment:2

This is not critical.

@kcrisman

This comment has been minimized.

@kcrisman
Copy link
Member

kcrisman commented Jul 7, 2012

comment:3

It turns out that #11839 was opened and did the first part of this ticket, including documenting the expand() issue.

However, the bug remains, so I'll just change this ticket to be about it.

@kcrisman kcrisman changed the title missing documentation and bug in collect bug in collect and/or term ordering in symbolics Jul 7, 2012
@burcin
Copy link

burcin commented Jul 7, 2012

comment:4

With the new description, this is probably a duplicate of #9880. I will check if the pynac changes for that ticket fix this one.

@zimmermann6
Copy link
Author

comment:5

bug still present in Sage 5.0.

Paul

@burcin
Copy link

burcin commented Jul 10, 2012

Reviewer: Burcin Erocal

@burcin
Copy link

burcin commented Jul 10, 2012

comment:6

This is a duplicate of #9880. With the Pynac patch queue and patches listed on #9880, I get:

sage: var('a b x y z')
(a, b, x, y, z)
sage: p = -a*x^3 - a*x*y^2 + 2*b*x^2*y + 2*y^3 + x^2*z + y^2*z + x^2 + y^2 + a>
sage: p.collect(x)
-a*x^3 + (2*b*y + z + 1)*x^2 + 2*y^3 + y^2*z - (a*y^2 - a)*x + y^2

We should close this after adding it as a doctest to #9880.

@burcin burcin removed this from the sage-5.1 milestone Jul 10, 2012
@burcin
Copy link

burcin commented Jul 27, 2012

comment:7

Doctest is in attachment: trac_9880-doctest_for_9046.patch:ticket:9880This can be closed now.

@zimmermann6
Copy link
Author

comment:8

Burcin,

first the ticket number is wrong (13107 instead of 9046) then the input p was mangled
(ends with a> instead of a*x).

Paul

@zimmermann6
Copy link
Author

Work Issues: correct doctests

@zimmermann6
Copy link
Author

Changed reviewer from Burcin Erocal to Burcin Erocal, Paul Zimmermann

@burcin
Copy link

burcin commented Nov 21, 2012

comment:9

I replaced the patch attached to #9880:

attachment: trac_9880-doctest_for_9046.patch:ticket:9880

I hope I got it right this time. Sorry for the noise.

@zimmermann6
Copy link
Author

comment:10

the patch is ok now. But since #9880 is not yet fixed, the doctest will fail. Thus we should wait for #9880 to review this one...

Paul

@KPanComputes
Copy link

Dependencies: #9880

@kcrisman
Copy link
Member

comment:12

That ticket has been merged, so I think this can be closed.

@jdemeyer
Copy link

Changed work issues from correct doctests to none

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

5 participants