Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
davidac897 opened this issue Jul 1, 2020 · 14 comments
Closed

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

davidac897 opened this issue Jul 1, 2020 · 14 comments

Comments

@davidac897
Copy link

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.

CC: @mstreng @sagetrac-kohel @sagetrac-florian @JRSijsling @jnoordsij

Component: algebra

Keywords: invariant theory

Author: Frédéric Chapoton

Branch/Commit: 7c7fc38

Reviewer: Sébastien Labbé

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

@davidac897 davidac897 added this to the sage-9.2 milestone Jul 1, 2020
@fchapoton
Copy link
Contributor

comment:1

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

@fchapoton
Copy link
Contributor

comment:2

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.

@fchapoton
Copy link
Contributor

New commits:

cfc52e9fix base_ring for some invariants

@fchapoton
Copy link
Contributor

Branch: u/chapoton/30035

@fchapoton
Copy link
Contributor

Commit: cfc52e9

@fchapoton
Copy link
Contributor

Changed author from jnoordsij to Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:4

This still needs a doctest.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 1, 2020

Changed commit from cfc52e9 to 7c7fc38

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 1, 2020

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

7c7fc38trac 30035 add indirect doctest

@fchapoton
Copy link
Contributor

comment:6

now with doctest

@fchapoton
Copy link
Contributor

comment:7

green bot, needs review

@fchapoton
Copy link
Contributor

comment:8

GREEN BOT, please review !

@seblabbe
Copy link
Contributor

Reviewer: Sébastien Labbé

@vbraun
Copy link
Member

vbraun commented Aug 23, 2020

Changed branch from u/chapoton/30035 to 7c7fc38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants