Remove genus2reduction

[jdemeyer]

genus2reduction has been ported to the PARI library and is available as the PARI function genus2red(). We should remove the package and use the new PARI function instead.

Depends on #15767

Component: packages: standard

Author: Jeroen Demeyer

Branch/Commit: f35c944

Reviewer: Peter Bruin

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

[jdemeyer]

Branch: u/jdemeyer/ticket/15808

[sagetrac-git]

Commit: 927e70c

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

927e70c

Remove genus2reduction

[jdemeyer]

Author: Jeroen Demeyer

[sagetrac-git]

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

701a3df

Upgrade to PARI git master

[sagetrac-git]

Changed commit from 927e70c to 701a3df

[sagetrac-git]

Changed commit from 701a3df to 3ffdf18

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

3ffdf18

Upgrade to PARI git master

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

[sagetrac-git]

Changed commit from 3ffdf18 to 927e70c

[fchapoton]

comment:10

There is an EXAMPLES: missing a double ::

[sagetrac-git]

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

11946d6

Fix documentation

[sagetrac-git]

Changed commit from 927e70c to 11946d6

[fchapoton]

Changed commit from 11946d6 to 1068fef

[fchapoton]

Changed branch from u/jdemeyer/ticket/15808 to public/ticket/15808

[fchapoton]

comment:12

There was a pyflakes warning about a missing definition. I made a correction in a function call for that reason.

Also made a few pep8 changes.

---

New commits:

c4f948d	Merge with 6.4.beta4
1068fef	trac #15808 correct name for _reduce + a few pep8 changes

[pjbruin]

comment:13

Do you have a reference for the following manipulations?

if minimal_equation.degree() == 5:
    minimal_disc *= minimal_equation[5]**2
# Multiply with suitable power of 2 of the form 2^(2*(d-1) - 12)
b = 2 * (minimal_equation.degree() - 1)
k = QQ((12 - minimal_disc.valuation(2), b)).ceil()
minimal_disc >>= 12 - b*k
minimal_disc = ZZ(minimal_disc)

Is the power of 2 in fact important, given that this is supposed to output the minimal discriminant away from 2?

[jdemeyer]

comment:14

Replying to @pjbruin:

Do you have a reference for the following manipulations?

They are based on looking at the PARI source code. But I must admit that I don't understand everything 100% clearly.

The line

if minimal_equation.degree() == 5:
    minimal_disc *= minimal_equation[5]**2

is based on

  RgX_to_6(polr, &a0,&a1,&a2,&a3,&a4,&a5,&a6);
  I.j10 = !signe(a0)? mulii(sqri(a1), ZX_disc(polr)): ZX_disc(polr);

The factor 2^(-12) comes from

I.j10 = gmul2n(I.j10,-12);

and the multiplication by 2^(2*(d-1)) comes from

dd = polval(polr,gen_2) & (~1); /* = 2 floor(val/2) */
polr = gmul2n(polr, -dd);

which multiplies the whole polynomial by a power of 4.
Multiplying a polynomial of degree d by a constant c multiplies the discriminant by c^(d-1).

Is the power of 2 in fact important?

I have no idea. I tried to reproduce as best as possible the output of the old genus2reduction program. That is also the reason why I emulated the raw() method and wrote to divisors_to_string() function: to check for myself if the new output was the same as the old output. Modulo some small details, this seems indeed to be the case.

[pjbruin]

comment:15

I'm not absolutely sure either, but from the last bit of PARI code you quoted, the following change seems to stay closer to the original:

diff --git a/src/sage/interfaces/genus2reduction.py b/src/sage/interfaces/genus2reduction.py
index 73d41d6..01c4019 100644
--- a/src/sage/interfaces/genus2reduction.py
+++ b/src/sage/interfaces/genus2reduction.py
@@ -514,9 +514,9 @@ class Genus2reduction(SageObject):
         minimal_disc = QQ(res[2].poldisc()).abs()
         if minimal_equation.degree() == 5:
             minimal_disc *= minimal_equation[5]**2
-        # Multiply with suitable power of 2 of the form 2^(2*(d-1) - 12)
+        # Multiply with suitable power of 2 of the form 2^(2*d - 12)
         b = 2 * (minimal_equation.degree() - 1)
-        k = QQ((12 - minimal_disc.valuation(2), b)).ceil()
+        k = min(a.valuation(2) for a in Q**2 + 4*P) & ~1
         minimal_disc >>= 12 - b*k
         minimal_disc = ZZ(minimal_disc)
 

This does not affect any doctests in genus2reduction.py.

[pjbruin]

comment:16

Actually my version above is certainly wrong, because it is not invariant under scaling of the variables in the original equation. However, your formula seems to mean that the 2-adic valuation of the minimal discriminant is bounded on the set of all genus 2 curves, which I can hardly believe, since it isn't even true for elliptic curves.

[pjbruin]

comment:17

We can also decide to give up trying to imitate the original program perfectly and allow the power of 2 to be different. I am attaching a patch that does this.

I also think you used an incorrect criterion to decide when the 2-part of the conductor was not computed. For example, one of the current doctests has the line

Conductor (away from 2): 954

Note that the "away from 2" part should only be there if the returned conductor is odd. According to the PARI documentation, we should check if 2 appears to the power -1 in the factorisation (fake at 2 in this case) of the conductor. This is fixed by the last part of the patch.

[jdemeyer]

comment:18

Attachment: discriminant.patch.gz

What you say (and your patch) makes sense.

[jdemeyer]

comment:19

Replying to @pjbruin:

However, your formula seems to mean that the 2-adic valuation of the minimal discriminant is bounded on the set of all genus 2 curves, which I can hardly believe, since it isn't even true for elliptic curves.

After thinking about this some more, perhaps my code is correct but it depends on the interpretation of "Minimal Discriminant (away from 2)". If you interpret it as "smallest positive integer which occurs as discriminant of a model of the given curve over Z[1/2]", then my code might be correct.

[jdemeyer]

Changed branch from public/ticket/15808 to u/jdemeyer/ticket/15808

[jdemeyer]

Changed commit from 1068fef to f35c944

[jdemeyer]

New commits:

f35c944

Fix "away from 2" check

[pjbruin]

Reviewer: Peter Bruin

[pjbruin]

comment:22

Replying to @jdemeyer:

Replying to @pjbruin:

However, your formula seems to mean that the 2-adic valuation of the minimal discriminant is bounded on the set of all genus 2 curves, which I can hardly believe, since it isn't even true for elliptic curves.

After thinking about this some more, perhaps my code is correct but it depends on the interpretation of "Minimal Discriminant (away from 2)". If you interpret it as "smallest positive integer which occurs as discriminant of a model of the given curve over Z[1/2]", then my code might be correct.

Possibly, but I would be surprised if this definition coincides with the one used by the original genus2reduction program. Anyway, I don't think it makes sense to spend too much time on finding a nice definition for "minimal discriminant away from 2", so any power of 2 in the "minimal discriminant away from 2" is fine with me.

[vbraun]

Changed branch from u/jdemeyer/ticket/15808 to f35c944