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

EC/ECNF consistency in local data display #6287

Merged
merged 2 commits into from
Dec 6, 2024

Conversation

JohnCremona
Copy link
Member

This fixes some points raised at #6275, making the display of local data consistent between ECQ and ECNF: table headings use the same notation, and ECNF now also has te text above the table saying whether or not the curve is semistable, and how many bad primes are. (Note that, as before, the table has a row for any primes where the curve has good reduction but the model does not. Such primes exist when there is no global minimal model; the model we then use has exactly one such prime.)

I also renamed "Period" --> "Global period" for ECNF a that is what the knowl defines, but ECQ still calls it "Real period" as that is what it is always called over Q (and there is a specific knowl too).

@JohnCremona
Copy link
Member Author

I do not know why in the ECNF local data table the headings of the columns for the prime and Reduction type are not centred. A fix for that would be welcome.

@roed314
Copy link
Contributor

roed314 commented Dec 5, 2024

All of the headings are left aligned (which is the standard for ntdata tables), it's just that those are the only ones where the width of the data is large enough that you can see it. We have this in style.css:

table.ntdata.centered th, table.ntdata.centered td {
  text-align: center;
}

so if you add centered as a class to the table it will do what you want.

Otherwise it looks good, so feel free to merge after you decide what to do about the centering! Thanks for fixing these.

@JohnCremona
Copy link
Member Author

All of the headings are left aligned (which is the standard for ntdata tables), it's just that those are the only ones where the width of the data is large enough that you can see it. We have this in style.css:

table.ntdata.centered th, table.ntdata.centered td {
  text-align: center;
}

so if you add centered as a class to the table it will do what you want.

Otherwise it looks good, so feel free to merge after you decide what to do about the centering! Thanks for fixing these.

Thanks, I will try that. CSS remains a dark art to me!

@roed314
Copy link
Contributor

roed314 commented Dec 5, 2024

If you change line 366 of ecnf-curve.html to <table class="ntdata centered"><thead> it centers everything.

I also noticed that $$\mathfrak{D}$$ is not defined (it looks like we use $$(\Delta)$$ above), and $$\mathfrak{N}$$ is capitalized in the local data table but not in the invariants section (where it is displayed as $$\mathfrak{n}$$).

@JohnCremona
Copy link
Member Author

Thanks for this! I haven't yet mmade any changes to this as I have been working on displaying th BSD formula for CNF and have just got it working (including the adjustment to the global period at complex places as explained in #5409).

I will finish this one off and make sure the other one is good and make another PR for that tomorrow (Friday).

@roed314 roed314 self-requested a review December 5, 2024 17:33
@JohnCremona
Copy link
Member Author

If you change line 366 of ecnf-curve.html to <table class="ntdata centered"><thead> it centers everything.

I also noticed that D is not defined (it looks like we use ( Δ ) above), and N is capitalized in the local data table but not in the invariants section (where it is displayed as n ).

  • Centering now fixed.
  • I changed the conductor ideal notation to upper case (i.e. \frak{N}) in all places rather the lower case.
  • Discriminants: there is a distinction between the discriminant $\Delta$ of the model, which is a field element, and the minimal discrminant, which is an ideal. We have been displaying the former in all cases, as a principal ideal $(\Delta)$ rather than as an element so that te factorization which is also displayed does not have an uninteresting unit factor; and only also displaying the minimal discriminant ideal and its factorisation when that is different, which is precisely when there is no global minimal model. And in such a case there is/was no line in the Invariants table with frak{D}_min, so its appearance in the local data table is mysterious, as you saw.
    I tried different ways to deal with this. The final version has an extra row in the Invariants table labelled "Discrimimant" which shows the discriminant of the model, with no factorization, with the knowl ec.discriminant defining that. When there is a global minimal model there are two more lines, for the "Discriminant ideal" and its norm, showing the principal ideal (Delta) and its factorization, and including " = D_min". When there is no global minimal model there are four lines, two for the "Discriminant ideal" and its norm, with their factorizations, and two for the minimal discriminant ideal and its factorization. In the former case the knowl for "Discriminant ideal" is ec.minimal_discrimimant; in the latter, the knowl for "Discriminant ideal" is ec.discriminant while that for "Minimal discriminant ideal" is ec.minimal_discriminant. And I added to that knowl (ec.minimial_discriminant) to say when it is just (Delta) and how it differs when not.

Phew. I have probably missed something, so another pairs of eyes would be welcome! I do think it instructive to see and compare the ideal factorisations when the model is not a global minimal one, so you can see the extra 12th power.

PR update in a moment...

@JohnCremona
Copy link
Member Author

@roed314
Copy link
Contributor

roed314 commented Dec 6, 2024

Looks good.

@roed314 roed314 merged commit 7eb05bb into LMFDB:main Dec 6, 2024
13 checks passed
@JohnCremona JohnCremona deleted the EC-local-data branch December 7, 2024 14:41
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