Bug in is_hyperbolic

[sagetrac-pkoprowski]

The method is_hyperbolic in the class of quadratic form returns incorrect results over the field QQ_2. Here is an explicit example:

q = DiagonalQuadraticForm(QQ, [1,1,-1,-1])
print q.is_hyperbolic(2)

The form <1,1,-1,-1> is clearly hyperbolic - by definition. It is a sum of two hyperbolic planes. Nevertheless Sage returns False here.

The reason is as follows. In the file quadratic_forms/quadratic_form__local_field_invariants.py in function is_hyperbolic the actual code is

    elif p == 2:
        return QQ(self.det() * (-1)**m).is_padic_square(p) and (self.hasse_invariant(p) == (-1)**m)    ## Actually, this -1 is the Hilbert symbol (-1,-1)_p

while it should be

    elif p == 2:
        return QQ(self.det() * (-1)**m).is_padic_square(p) and (self.hasse_invariant(p) == (-1)**(m*(m-1)/2))    ## Actually, this -1 is the Hilbert symbol (-1,-1)_p

For mathematical explanation see e.g. T.Y. Lam "Introduction to Quadratic Forms over Field", Proposition V.3.25

I'm not sure how to formally patch the code, so I'm posting it this way.

Component: quadratic forms

Author: Malcolm Rupert

Branch/Commit: f99393f

Reviewer: Frédéric Chapoton, Travis Scrimshaw

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

[sagetrac-MRupert]

Branch: u/MRupert/bug_in_is_hyperbolic

[sagetrac-MRupert]

comment:2

This commit should fix the problem. I also added functionality and improved the documentation on the infinite place.

---

New commits:

abb9f99

18430: Fixes self.is_hyperbolic(2) and various cleanups

[sagetrac-MRupert]

Commit: abb9f99

[sagetrac-MRupert]

Author: Malcolm Rupert

[jdemeyer]

comment:3

Can you replace

if p == "infinity":
    return self.is_definite()
else:
    ...

by

if p == Infinity:
    return self.is_definite()

...

This means you don't need to indent the whole block for p a prime number and I also prefer the actual value Infinity (which needs to be imported from sage.rings.infinity) instead of the string "infinity".

I also don't understand why you use -1 at one point and "infinity" somewhere else.

And instead of writing

(-1)**(m*(m-1)/2)) ## Actually, this -1 is the Hilbert symbol (-1,-1)

why don't you actually write

hilbert_symbol(-1, -1, p)**(m*(m-1)/2))

[fchapoton]

Changed commit from abb9f99 to fc968e3

[fchapoton]

comment:5

rebase and clean-up

---

New commits:

fc968e3

Merge branch 'u/MRupert/bug_in_is_hyperbolic' in 7.4.b2

[fchapoton]

Changed branch from u/MRupert/bug_in_is_hyperbolic to u/chapoton/18430

[sagetrac-git]

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

64cfdf3	Merge branch 'u/chapoton/18430' in 8.0.b0
7690f84	trac 18430 fixing doctests, plus little code cleanup

[sagetrac-git]

Changed commit from fc968e3 to 7690f84

[tscrim]

comment:9

Replying to @jdemeyer:

I also don't understand why you use -1 at one point and "infinity" somewhere else.

And instead of writing

(-1)**(m*(m-1)/2)) ## Actually, this -1 is the Hilbert symbol (-1,-1)

why don't you actually write

hilbert_symbol(-1, -1, p)**(m*(m-1)/2))

I am pretty sure the former is (much) faster and the comment makes the simplification clear.

[sagetrac-git]

Changed commit from 7690f84 to 3231a6d

[sagetrac-git]

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

3231a6d

trac 18430 details

[fchapoton]

comment:11

done

[fchapoton]

comment:12

ping ?

[tscrim]

Reviewer: Frédéric Chapoton, Travis Scrimshaw

[tscrim]

comment:13

If you just fix up this INPUT: docstring (in a few places):

-`p` -- a prime number > 0 or `-1` for the infinite place.
+- `p` -- a prime number > 0 or `-1` for the infinite place

Once that is done, you can set a positive review on my behalf.

[sagetrac-pmercuri]

Work Issues: documentation formatting

[sagetrac-git]

Changed commit from 3231a6d to f99393f

[sagetrac-git]

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

b4f668a	Merge branch 'u/chapoton/18430' in 8.0.b9
f99393f	trac 18430 INPUT field has no dot

[fchapoton]

Changed work issues from documentation formatting to none

[tscrim]

comment:17

LGTM.

[vbraun]

Changed branch from u/chapoton/18430 to f99393f