Opened 11 years ago

Closed 8 years ago

#9046 closed defect (duplicate)

bug in collect and/or term ordering in symbolics

Reported by: zimmerma Owned by: burcin
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: symbolics Keywords: pynac
Cc: kcrisman Merged in:
Authors: Reviewers: Burcin Erocal, Paul Zimmermann
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #9880 Stopgaps:

Status badges

Description (last modified by kcrisman)

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

Change History (13)

comment:1 Changed 11 years ago by burcin

  • Keywords pynac added

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.

comment:2 Changed 10 years ago by kcrisman

  • Cc kcrisman added
  • Priority changed from critical to minor

This is not critical.

comment:3 Changed 9 years ago by kcrisman

  • Component changed from calculus to symbolics
  • Description modified (diff)
  • Summary changed from missing documentation and bug in collect to bug in collect and/or term ordering in symbolics

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.

comment:4 Changed 9 years ago by burcin

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

comment:5 Changed 9 years ago by zimmerma

bug still present in Sage 5.0.

Paul

comment:6 Changed 9 years ago by burcin

  • Milestone changed from sage-5.1 to sage-duplicate/invalid/wontfix
  • Reviewers set to Burcin Erocal
  • Status changed from new to needs_review

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.

comment:7 Changed 9 years ago by burcin

  • Status changed from needs_review to positive_review

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

comment:8 Changed 9 years ago by zimmerma

  • Reviewers changed from Burcin Erocal to Burcin Erocal, Paul Zimmermann
  • Status changed from positive_review to needs_work
  • Work issues set to correct doctests

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

comment:9 Changed 9 years ago by burcin

  • Status changed from needs_work to needs_review

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.

comment:10 Changed 9 years ago by zimmerma

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

comment:11 Changed 8 years ago by knsam

  • Dependencies set to #9880

comment:12 Changed 8 years ago by kcrisman

  • Status changed from needs_review to positive_review

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

comment:13 Changed 8 years ago by jdemeyer

  • Resolution set to duplicate
  • Status changed from positive_review to closed
  • Work issues correct doctests deleted
Note: See TracTickets for help on using tickets.