sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.37k stars 462 forks source link

Follow up to #1940: Ideal comparison improvements #1952

Closed 85eec1a4-3d04-4b4d-b711-d4db03337c41 closed 16 years ago

85eec1a4-3d04-4b4d-b711-d4db03337c41 commented 16 years ago
[21:30] <wstein-1649> one thing about #1940...
[21:30] <wstein-1649> If you're comparing two ideals, it might make sense to use the term ordering for which
[21:30] <wstein-1649> computing groebner basis is *easy*.
[21:31] <wstein-1649> E.g., wrt one order it can be super super hard, and wrt to another, quite easy.
[21:31] <mabshoff> Yep. That sounds like a great idea.
[21:31] <mabshoff> i.e. degrevlex per default.
[21:31] <mabshoff> Ticket?
[21:31] <wstein-1649> But in malb's code for #1940 he just uses whatever term order.
[21:31] <mabshoff> Well, I think he caches the gbasis, so in some cases it might already exist.
[21:32] <wstein-1649> Also, he doesn't even seem to check that other is an ideal.
[21:32] <wstein-1649> Also, if the two ideals are in the same ring but with two different term orders,
[21:32] <wstein-1649> that __cmp__ function will get the wrong answer.
[21:32] <mabshoff> mmh.
[21:32] <mabshoff> I guess reopen, but leave the patch applied?
[21:32] <wstein-1649> I think his patch perfectly addresses the given #1940.
[21:32] <wstein-1649> But there should be another trac ticket about my points above.
[21:33] <wstein-1649> Yeah?
[21:33] <mabshoff> Yep.
[21:33] <wstein-1649> Can you open it and maybe paste this discussion in?
[21:33] <mabshoff> Already on the way :)
[21:33] <wstein-1649> cool.

Component: commutative algebra

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

williamstein commented 16 years ago
comment:1
cwitty: wstein-1649, what do you mean "in the same ring but with two different term orders"?  Doesn't Sage treat the term order as part of the ring?
[3:46pm] wstein-1649: yes.
[3:47pm] wstein-1649: cwitty -- what if I make two QQ[x,y]'s with two different term orders.  But then I define ideals I, J generated by (x,y) and (x,y).
[3:47pm] wstein-1649: Shouldn't I == J be true?
[3:47pm] wstein-1649: But with Malb's code, it would be false, sort of by accident.
[3:48pm] cwitty: I guess I don't really care whether I==J is true or not.
[3:48pm] wstein-1649: ?

Maybe I'm just confused. In any case, some further thought here would be a good idea. Mainly relevant is optimization.

williamstein commented 16 years ago
comment:2

This -- maybe (?) caused by #1940 -- is very inconsistent:

sage: R = PolynomialRing(QQ, 'x,y,z', order='degrevlex'); R
Multivariate Polynomial Ring in x, y, z over Rational Field
sage: S = PolynomialRing(QQ, 'x,y,z', order='invlex'); S
Multivariate Polynomial Ring in x, y, z over Rational Field
sage: I = R.ideal([R.0,R.1])
sage: J = S.ideal([S.0,S.1])
sage: I == J
True
sage: cmp(I,J)
1
sage: I.__cmp__(J)
1
85eec1a4-3d04-4b4d-b711-d4db03337c41 commented 16 years ago
comment:3

It is cause by #1940: Compare the same session in 2.10 vanilla:

----------------------------------------------------------------------
| SAGE Version 2.10, Release Date: 2008-01-18                        |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------

sage: R = PolynomialRing(QQ, 'x,y,z', order='degrevlex'); R
Multivariate Polynomial Ring in x, y, z over Rational Field
sage: S = PolynomialRing(QQ, 'x,y,z', order='invlex'); S
Multivariate Polynomial Ring in x, y, z over Rational Field
sage: I = R.ideal([R.0,R.1])
sage: J = S.ideal([S.0,S.1])
sage: I==J
True
sage: cmp(I,J)
0
sage: I.__cmp__(J)
0
sage:

Cheers,

Michael

85eec1a4-3d04-4b4d-b711-d4db03337c41 commented 16 years ago
comment:4

1940 also seems to have broken the toy_buchberger doctest:

sage -t  devel/sage-main/sage/rings/polynomial/toy_buchberger.py
**********************************************************************
File "toy_buchberger.py", line 60:
    sage: I = sage.rings.ideal.Katsura(P)
Expected:
    // sage334              [0]  ideal, 3 generator(s)
Got:
    // sage310              [0]  ideal, 3 generator(s)
**********************************************************************
1 items had failures:
   1 of  16 in __main__.example_0
***Test Failed*** 1 failures.
For whitespace errors, see the file .doctest_toy_buchberger.py
         [4.1 s]
exit code: 256

----------------------------------------------------------------------
The following tests failed:

Cheers,

Michael

williamstein commented 16 years ago
comment:5

In

    // sage310              [0]  ideal, 3 generator(s)

the variable name just depends on how many times Singular has been called, since adding any doctest that involves Singular's pexpect interface in will break everything!
It's stupid-ish to even list it. It would be much better to change the above to

    // sage...             [0]  ideal, 3 generator(s)
85eec1a4-3d04-4b4d-b711-d4db03337c41 commented 16 years ago
comment:6

William: Yep, your fix is the obvious correct one. I fixed it in my repo and attached the patch here.

85eec1a4-3d04-4b4d-b711-d4db03337c41 commented 16 years ago
comment:8

Since the doctest patch has been merged change the Summary line for now.

Cheers,

Michael

malb commented 16 years ago
comment:9

Attachment: mpolynomial_ideal_refactor.patch.gz

The attached patch fixes that issue and also refactors caching of Gröbner bases.

malb commented 16 years ago
comment:10

was, can I ask you to review my patch?

mwhansen commented 16 years ago

Attachment: trac_1952-2.patch.gz

mwhansen commented 16 years ago
comment:11

Positive review for malb's changes. Now, my latest patch needs to be looked at.

85eec1a4-3d04-4b4d-b711-d4db03337c41 commented 16 years ago
comment:12

There is some slight trouble in tut.tex with both patches applied (I deleted the one line doctest fix I added a while ago):

sage -t -long devel/doc/tut/tut.tex
**********************************************************************
File "/scratch/mabshoff/release-cycle/sage-3.1.2.alpha1/tmp/tut.py", line 2181:
    : D.irreducible_components()
Expected:
    [
    Closed subscheme of Affine Space of dimension 2 over Rational Field defined
    by:
      x^3 + y^3 - 1,
    Closed subscheme of Affine Space of dimension 2 over Rational Field defined
    by:
      x^2 + y^2 - 1
    ]
Got:
    [
    Closed subscheme of Affine Space of dimension 2 over Rational Field defined by:
      x^2 + y^2 - 1,
    Closed subscheme of Affine Space of dimension 2 over Rational Field defined by:
      x^3 + y^3 - 1
    ]
**********************************************************************
File "/scratch/mabshoff/release-cycle/sage-3.1.2.alpha1/tmp/tut.py", line 2197:
    : V.irreducible_components()
Expected:
    [
    Closed subscheme of Affine Space of dimension 2 over Rational Field defined
    by:
      x + y + 2
      2*y^2 + 4*y + 3,
    Closed subscheme of Affine Space of dimension 2 over Rational Field defined
    by:
      y - 1
      x,
    Closed subscheme of Affine Space of dimension 2 over Rational Field defined
    by:
      y
      x - 1
    ]
Got:
    [
    Closed subscheme of Affine Space of dimension 2 over Rational Field defined by:
      y
      x - 1,
    Closed subscheme of Affine Space of dimension 2 over Rational Field defined by:
      y - 1
      x,
    Closed subscheme of Affine Space of dimension 2 over Rational Field defined by:
      x + y + 2
      2*y^2 + 4*y + 3
    ]
**********************************************************************
malb commented 16 years ago
comment:13

Attachment: trac_1952-tutfix.patch.gz

Apply all three patches in order (last one goes to doc repo).

85eec1a4-3d04-4b4d-b711-d4db03337c41 commented 16 years ago
comment:14

Merged all three patches in Sage 3.1.2.alpha2