Opened 9 years ago

Closed 9 years ago

#13451 closed enhancement (fixed)

Classical invariant theory

Reported by: vbraun Owned by: AlexGhitza
Priority: major Milestone: sage-5.7
Component: algebra Keywords:
Cc: dimpase, cremona Merged in: sage-5.7.beta1
Authors: Volker Braun Reviewers: Dmitrii Pasechnik
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by vbraun)

Even though the field fell out of fashion a century ago, sometimes it is useful to have the formula for invariants of algebraic forms. This ticket implements some a basic framework, and in that the binary quartic and ternary cubic.

Apply trac_13451_invariant_theory.patch

Attachments (1)

trac_13451_invariant_theory.patch (66.8 KB) - added by vbraun 9 years ago.
Updated patch

Download all attachments as: .zip

Change History (25)

comment:1 Changed 9 years ago by vbraun

  • Cc dimpase cremona added
  • Status changed from new to needs_review

comment:2 Changed 9 years ago by dimpase

IMHO, the docstring "algebraic forms" needs a bit of explanation, something like "algebraic forms, i.e., homogeneous polynomials"

comment:3 follow-up: Changed 9 years ago by vbraun

AlgebraicForm an abstract base class, you'll never get to see its docstring unless you want to extend the module. So I don't see any point in being pedagogical there. If you never heard the term and can't be bothered to google it then you shouldn't extend this module.

comment:4 in reply to: ↑ 3 Changed 9 years ago by dimpase

Replying to vbraun:

AlgebraicForm an abstract base class, you'll never get to see its docstring unless you want to extend the module. So I don't see any point in being pedagogical there. If you never heard the term and can't be bothered to google it then you shouldn't extend this module.

google, schmoogle... :–)

it's not about being pedagogical, it's about being understood. I have never seen the expression "algebraic form" used to mean what you mean, and I suppose many mathematicians would share my point of view here.

comment:5 Changed 9 years ago by vbraun

OK, added.

comment:6 Changed 9 years ago by vbraun

  • Description modified (diff)

Of course I'm realistic enough to see that nobody is ever going to bother to review this ticket, but its still important for my work so I'll keep adding stuff :-P

comment:7 Changed 9 years ago by cremona

I'm very interested! And could use this, specifically binary quadratics and quartics and ternary cubics come up in 2- and 3-descent on elliptic curves. I just have too many other things right now, but I'm happy to see the updates being added.

comment:8 Changed 9 years ago by dimpase

I think the documentation of various parts of QuadraticForm() ought to be improved: one needs to explicitly define the order on monomials used in coeffs() (and monomials()). Somehow, coeffs()'s docs still talk about binary forms --- is it a leftover from the previous version?

As well, what about an alias coefficients() for coeffs() ?

Last edited 9 years ago by dimpase (previous) (diff)

comment:9 Changed 9 years ago by dimpase

I also think it should be placed into sage/rings rather than sage/schemes, as it deals with ring elements (of the corresponding ring of invariants).

comment:10 Changed 9 years ago by vbraun

  • Description modified (diff)
  • Reviewers set to Dmitrii Pasechnik

I've addressed Dima's corrections. Also, folded into one patch.

comment:11 Changed 9 years ago by vbraun

Patchbot didn't get the memo about the folded patch.

Apply trac_13451_invariant_theory.patch

comment:12 Changed 9 years ago by dimpase

  • Status changed from needs_review to positive_review

comment:13 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-5.6 to sage-5.7

comment:14 follow-up: Changed 9 years ago by jdemeyer

Cool :-)

        ary = ['unary', 'binary', 'ternary', 'quaternary', 'quinary',
               'senary', 'septenary', 'octonary', 'nonary', 'denary']
        try:
            s += ary[self._n-1]
        except IndexError:
            s += 'algebraic'
        ic = ['monic', 'quadratic', 'cubic', 'quartic', 'quintic',
              'sextic', 'septimic', 'octavic', 'nonic', 'decimic',
              'undecimic', 'duodecimic']
        s += ' '
        try:
            s += ic[self._d-1]
        except IndexError:
            s += 'form'

comment:15 in reply to: ↑ 14 Changed 9 years ago by dimpase

Replying to jdemeyer:

Cool :-)

        ary = ['unary', 'binary', 'ternary', 'quaternary', 'quinary',
               'senary', 'septenary', 'octonary', 'nonary', 'denary']
        try:
            s += ary[self._n-1]
        except IndexError:
            s += 'algebraic'
        ic = ['monic', 'quadratic', 'cubic', 'quartic', 'quintic',
              'sextic', 'septimic', 'octavic', 'nonic', 'decimic',
              'undecimic', 'duodecimic']
        s += ' '
        try:
            s += ic[self._d-1]
        except IndexError:
            s += 'form'

I actually think this should be documented in Latin...

comment:16 Changed 9 years ago by jdemeyer

LaTeX complains:

! Missing $ inserted.
<inserted text>
                $
l.717126 ...invariant_theory]{wp_invariant_theory}
                                                  {\phantomsection\label{sag...

?
! Emergency stop.
<inserted text>
                $
l.717126 ...invariant_theory]{wp_invariant_theory}
                                                  {\phantomsection\label{sag...

!  ==> Fatal error occurred, no output PDF file produced!
Transcript written on reference.log.
make[1]: *** [reference.pdf] Error 1

comment:17 Changed 9 years ago by jdemeyer

Also, there is some dubious use of assert where raise ValueError would be more appropriate.

comment:18 Changed 9 years ago by jdemeyer

  • Status changed from positive_review to needs_work

Changed 9 years ago by vbraun

Updated patch

comment:19 follow-up: Changed 9 years ago by vbraun

The _check_covariant method containing the assertions are only used in testing the module, but not any user-visible functionality. I fixed the LaTeX error, printing the 8059 page reference manual now (j/k ;-)

comment:20 in reply to: ↑ 19 Changed 9 years ago by jdemeyer

Replying to vbraun:

printing the 8059 page reference manual now (j/k ;-)

Indeed, I always wondered why we bother with the PDF manual...

comment:21 Changed 9 years ago by dimpase

  • Status changed from needs_work to positive_review

Just finished checking the 8059 pages.

comment:22 Changed 9 years ago by nbruin

Just curious: What's the point of InvariantTheoryFactory? Is there a benefit to the routines in there being methods rather than just module-level functions?

comment:23 Changed 9 years ago by vbraun

I'm pretty sure you could do the same thing with module-level functions, there is little difference in Python between the two. You'd have to use an extra file to isolate the module, but you save yourself instatiating the class.

comment:24 Changed 9 years ago by jdemeyer

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