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

ECNF better display of invariants #6273

Merged
merged 2 commits into from
Nov 28, 2024
Merged

Conversation

JohnCremona
Copy link
Member

Dealing with #4943 for ECNF as already done for ECQ.

Complications to consider include:

  • sometimes we do not have rank data at all, sometimes just bounds, in which case instead of having r (in one column) = (in the next) and the value, I have r? in the first, nothing in the second, and "not available" or a range e.g. $0\le r\le 2$. I am open to other suggestions.
  • Under torsion structure the columns are (e.g.) | E(K)_tors | \cong | Z/3Z | so for when it is trivial I kept the isomorphism sign but changed "trivial" to "0" (this is set in the python code, all else here is in the template). Other suggestions?
  • There are lots of combinations depending on how much data we do (or do not) have. I hope I checked them all but please look at many random curves
  • The knowl for "Discriminant" gives the usual definition for a field element (from the Weierstrass model) but what is displayed is the ideal this generates, so I denoted it $(\Delta)$ not just $\Delta$. Perhaps the title of that row should be "discriminant ideal"? For curves with no global minimal model (for an example see the "some interesting curves" list) there are two more lines for the minimal discriminant, and that is always an ideal by definition.

@AndrewVSutherland
Copy link
Member

I think this looks good, but I did notice a few small things. Most of them are not directly caused this PR but I thought I'd mention them all here in case you decide you want to address them now, but you should feel totally free to just tell me to create an issue for anything you don't want to deal with now.

(1) "(no complex multiplication)" is in a different column than it is on the ECQ pages. It might be better to make it match the ECQ pages (this is a trivial change and seems worth doing now).

(2) The "Generators" invariant in the Mordell-Weil generators only lists generators for the free part, and this isn't consistent with the knowl (e.g. a Z-basis for https://beta.lmfdb.org/EllipticCurve/2.0.84.1/768.1/b/3 would have 5 elements, not 3). Possibly it should say "Non-torsion generators" and get a new knowl, but see (4) below. I notice this also impacts the ECQ pages.

(3) I notice that the ECQ/ECNF Mordell-Weil Invariants sections use rather different layouts, are there reasons to format them differently? I like that everything is grouped under one heading in the ECNF layout, but I also like the way the structure of the MW group is shown on the ECQ pages (including both the free and torsion parts rather than giving the structure of the torsion subgroup on a separate line). I think it might be worth figuring out the optimal layout (perhaps combining elements of both, but also see (4) below), and then uniformizing them.

(4) Related to (2) and (3), I notice that both ECQ/ECNF use a table with a column for each infinite order generator, which is already likely to force horizontal scrolling on pages like https://www.lmfdb.org/EllipticCurve/Q/19047851/a/1 for many users, and I worry things will get worse in the future if/when we add more higher rank curves (we have definitely been getting requests to include the new Elkies-Klagsbrun rank 29 curve, for example!) I wonder if it would make more sense to use a table with a row for each generator (which could also include torsion generators and a column for the order, giving you a complete Z-basis). This table will be very short for most curves, so I don't think screen real estate is a major concern.

@JohnCremona
Copy link
Member Author

Thanks @AndrewVSutherland for the detailed comments, all very sensible. I'll deal with (1) and (2) right away. It's good to try to make the ECQ and ECNF pages as consistent as possible as suggested in (3), that can be a new issue, as can (4).

@JohnCremona
Copy link
Member Author

The new commit addresses (1) and (2) and a few other small inconsistencies that I noticed. I enhanced the knowl for MW generators so that it defines torsion and non-torsion generators. I also decided that the knowl ec.q. canonical_height was redundant and not as good as ec.canonical_height, so replaced the places where the latter was used: a couple in the actual templates (changed in the latest commit) and 6 in bottom knowls. So it it would be great if these recently modified knowls were also reviewd. Thanks!

@AndrewVSutherland AndrewVSutherland merged commit 49cc378 into LMFDB:main Nov 28, 2024
13 checks passed
@AndrewVSutherland
Copy link
Member

Done!

@JohnCremona
Copy link
Member Author

Done!

Great, thanks. I am now working on the two others, which are based on this so that makes life simpler.

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

Successfully merging this pull request may close these issues.

2 participants