Opened 2 years ago

Closed 2 years ago

#30035 closed defect (fixed)

Invariants of Ternary Cubic Forms do not live in the base ring

Reported by: gh-davidac897 Owned by:
Priority: minor Milestone: sage-9.2
Component: algebra Keywords: invariant theory
Cc: Marco Streng, David Kohel, Florian Bouyer, Jeroen Sijsling, Jesper Noordsij Merged in:
Authors: Frédéric Chapoton Reviewers: Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: 7c7fc38 (Commits, GitHub, GitLab) Commit: 7c7fc38d2270fbc90b1015f15244ef4f733a5729
Dependencies: Stopgaps:

Status badges

Description

It seems that invariants of ternary cubic forms (and I assume other kinds of forms, but I didn't try) live in the polynomial ring, not in the base ring. Therefore, I get this, which shouldn't happen:

R.<a,b,c> = QQ[]
RR.<x,y,z> = QQ[]

f = 1994893*a^3 + 4498037*a^2*b + 3358044*a*b^2 + 830875*b^3 + 7859654*a^2*c + 11828845*a*b*c + 4420000*b^2*c + 10319781*a*c^2 + 7775375*b*c^2 + 4515625*c^3
ff = x^3 + 11*x^2*y - 14*x*y^2 + y^3 + 11*x^2*z + 135*x*y*z - 160*y^2*z - 14*x*z^2 + 3*y*z^2 + z^3


T = invariant_theory.ternary_cubic(f)
TT = invariant_theory.ternary_cubic(ff)

T.S_invariant(); TT.S_invariant(); T.S_invariant() == TT.S_invariant();  T.S_invariant() == QQ(TT.S_invariant())

produces outputs of -705911761/1296, -705911761/1296, False, True.

Change History (10)

comment:1 Changed 2 years ago by Frédéric Chapoton

The problem comes from scaled_coeffs:

sage: [parent(u) for u in T.scaled_coeffs()]
[Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field]

and then from coeffs

sage: [parent(u) for u in T.coeffs()]
[Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field,
 Multivariate Polynomial Ring in a, b, c over Rational Field]

which calls _extract_coefficients

Last edited 2 years ago by Frédéric Chapoton (previous) (diff)

comment:2 Changed 2 years ago by Frédéric Chapoton

This seems from deep inside the invariant code. The choice not to separate true variables of the cubic forms and variables for the coefficients seems to be very wrong, and a serious flaw in the design.

comment:3 Changed 2 years ago by Frédéric Chapoton

Authors: jnoordsijFrédéric Chapoton
Branch: u/chapoton/30035
Cc: Jesper Noordsij added
Commit: cfc52e9e2f080eb37b7496126ad6b2e8fad88f77
Status: newneeds_review

New commits:

cfc52e9fix base_ring for some invariants

comment:4 Changed 2 years ago by Frédéric Chapoton

This still needs a doctest.

comment:5 Changed 2 years ago by git

Commit: cfc52e9e2f080eb37b7496126ad6b2e8fad88f777c7fc38d2270fbc90b1015f15244ef4f733a5729

Branch pushed to git repo; I updated commit sha1. New commits:

7c7fc38trac 30035 add indirect doctest

comment:6 Changed 2 years ago by Frédéric Chapoton

now with doctest

comment:7 Changed 2 years ago by Frédéric Chapoton

Cc: Marco Streng David Kohel Florian Bouyer Jeroen Sijsling added

green bot, needs review

comment:8 Changed 2 years ago by Frédéric Chapoton

GREEN BOT, please review !

comment:9 Changed 2 years ago by Sébastien Labbé

Reviewers: Sébastien Labbé
Status: needs_reviewpositive_review

comment:10 Changed 2 years ago by Volker Braun

Branch: u/chapoton/300357c7fc38d2270fbc90b1015f15244ef4f733a5729
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.