Opened 11 years ago

Closed 10 years ago

#8909 closed enhancement (fixed)

Coercion from Gap to cyclotomic fields; usage of GAP to improve computation of invariant rings

Reported by: SimonKing Owned by: was
Priority: major Milestone: sage-4.5.2
Component: interfaces Keywords: gap, cyclotomic fields, invariant rings
Cc: wdj Merged in: sage-4.5.2.alpha0
Authors: Simon King Reviewers: David Loeffler, Mike Hansen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

When coercing from GAP to a cyclotomic field, it was assumed that the generator of a cyclotomic field is always called E(n). But this is not necessarily the case, in particular when the object in GAP was created from Sage.

Moreover, GAP prints an additional exclamation mark in front of numbers if they are part of a matrix defined over a cyclotomic field.

For these two reasons, the following example used to fail, but now works with the patch:

            sage: F=CyclotomicField(8)
            sage: z=F.gen()
            sage: a=gap(z+1/z); a
            -zeta8^3+zeta8
            sage: F(a)
            -zeta8^3 + zeta8
            sage: b=gap(Matrix(F,[[z^2,1],[0,a+1]])); b
            [ [ zeta8^2, !1 ], [ !0, -zeta8^3+zeta8+1 ] ]
            sage: b[1,2]
            !1
            sage: F(b[1,2])
            1
            sage: Matrix(b,F)
            [             zeta8^2                    1]
            [                   0 -zeta8^3 + zeta8 + 1]

The idea was

  • to remove the exclamation mark when it is attempted to coerce into the rationals
  • to test whether the generator name in GAP happens to coincide with the generator name in Sage (here: zeta8).

The motivation for working on it is my attempt to improve the computation of non-modular invariant rings of finite groups: There is a doc test using a finite matrix group over a cyclotomic field.

One massive bottle neck for the computation of invariant rings with Singular is the computation of the Reynolds operator. It requires to enumerate the group elements, and Singular is not good at this task.

The patch uses GAP to enumerate the group elements and uses this to construct the Reynolds operator in Singular. For complicated groups, this should save a massive amount of resources.

With the patch, the enumeration of group elements in Singular has the status of a backup: If the transformation of the matrix group into GAP fails or if the transformation of the resulting GAP matrices back into Sage fails, then the old algorithm is used.

I think this ticket is about "interfaces". I hope this labelling is correct.

Attachments (2)

8909_gap2cyclotomic.patch (6.2 KB) - added by SimonKing 11 years ago.
Improve coercion from GAP into cyclotomic fields; use GAP to compute Reynolds operators for Singular
trac_8909_catch_exception.patch (1020 bytes) - added by SimonKing 10 years ago.
Specify an exception to be caught

Download all attachments as: .zip

Change History (10)

Changed 11 years ago by SimonKing

Improve coercion from GAP into cyclotomic fields; use GAP to compute Reynolds operators for Singular

comment:1 Changed 11 years ago by SimonKing

  • Status changed from new to needs_review

Sorry, I forgot to label it "needs review"

comment:2 Changed 10 years ago by davidloeffler

When this is merged we can probably close #5618 too.

comment:3 Changed 10 years ago by davidloeffler

  • Status changed from needs_review to positive_review

Patch applies fine to 4.5.alpha1, code looks plausible, and all doctests pass. Contrary to my earlier hope, however, it does not fix #5618 which seems to be a separate problem.

comment:4 follow-up: Changed 10 years ago by mhansen

  • Reviewers set to David Loeffler, Mike Hansen
  • Status changed from positive_review to needs_work

There's a bare except clause at 6767 which should be fixed.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 10 years ago by SimonKing

Replying to mhansen:

There's a bare except clause at 6767 which should be fixed.

By 'bare', you mean that it is not specified what error is raised? So, except TypeError: instead of except:? Well this would be easy enough to fix.

Changed 10 years ago by SimonKing

Specify an exception to be caught

comment:6 in reply to: ↑ 5 Changed 10 years ago by SimonKing

  • Status changed from needs_work to needs_review

Replying to SimonKing:

Replying to mhansen:

There's a bare except clause at 6767 which should be fixed.

By 'bare', you mean that it is not specified what error is raised? So, except TypeError: instead of except:? Well this would be easy enough to fix.

Under the assumption that I understood you correctly, I provided a second patch that specifies that we only want to catch a TypeError, and return to needs_review.

comment:7 Changed 10 years ago by mhansen

  • Status changed from needs_review to positive_review

Yep, looks good to me. If you didn't specify that, the except clause would also catch things like KeyboardInterrupt? (Ctrl-C) which probably isn't intended.

Apply both patches.

comment:8 Changed 10 years ago by mpatel

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