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

more efficient method for number of real components of an elliptic curve over Q #31433

Closed
JohnCremona opened this issue Feb 25, 2021 · 11 comments

Comments

@JohnCremona
Copy link
Member

Define an elliptic curve, find its discriminant
and its number of real components:

sage: E = EllipticCurve([0, -1, 1, 0, 0])
sage: E.discriminant()
-11
sage: E.real_components()
1

The number of real components is 1 or 2 when the discriminant is negative or positive respectively. The current code does a lot of unnecessary work:

        invs = self.short_weierstrass_model().ainvs()
        x = rings.polygen(self.base_ring())
        f = x**3 + invs[3]*x + invs[4]
        if f.discriminant() > 0:
            return 2
        else:
            return 1

It is unnecessary to compute a short Weierstrass model, take its coefficients, construct a polynomial, and compute its discriminant, since E.discriminant() has the same sign as the discriminant computed here.

As well as fixing this we add a real_components() method for elliptic curves over number fields, which takes as a parameter a real embedding of the base field.

CC: @slel

Component: elliptic curves

Author: John Cremona

Branch/Commit: 41b446c

Reviewer: Frédéric Chapoton

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

@JohnCremona JohnCremona added this to the sage-9.3 milestone Feb 25, 2021
@JohnCremona
Copy link
Member Author

Commit: 2f0b0c1

@JohnCremona
Copy link
Member Author

New commits:

f3c6f63#31433: simplify real_components method for elliptic curves over QQ
2f0b0c1#31433: add real_components method for elliptic curves over number fields

@JohnCremona
Copy link
Member Author

Branch: u/cremona/31433

@slel

This comment has been minimized.

@fchapoton
Copy link
Contributor

comment:4

two many final dots here:

+        Return the number of real components with respect to a real embedding of the base field..

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 26, 2021

Changed commit from 2f0b0c1 to 41b446c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 26, 2021

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

41b446c#31433: delete extra dot

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:6

ok, looks good. Avanti !

@JohnCremona
Copy link
Member Author

comment:7

Replying to @fchapoton:

ok, looks good. Avanti !

Merci!

@vbraun
Copy link
Member

vbraun commented Mar 9, 2021

Changed branch from u/cremona/31433 to 41b446c

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