# Ticket #6106(needs_work enhancement)

Opened 4 years ago

## [with patch, needs work] Additional functions for Indefinite Binary Quadratic Forms

Reported by: Owned by: aliashamieh justin minor sage-5.11 quadratic forms binary quadratic forms ncalexan John Cremona, Nick Alexander Alia Hamieh

### Description

I added functions to check for the reducibility of an indefinite binary quadratic form and reduce it.

## Change History

### comment:1 Changed 4 years ago by davidloeffler

• Summary changed from [with patch, need review] Addition Indefinite Binary Quadratic Forms to [with patch, needs review] Addition Indefinite Binary Quadratic Forms

Am changing this from "need review" to "needs review" so it shows up in the standard queries

### comment:2 Changed 4 years ago by cremona

• Summary changed from [with patch, needs review] Addition Indefinite Binary Quadratic Forms to [with patch, with review, needs work] Additional functions for Indefinite Binary Quadratic Forms

Review

The patch applied fine to 4.0.1, and it provides some functions which look useful.

I think that not all new new functions have doctests. Why are the new functions not implemented as member functions of the BinaryQF class? That would be much better; as it is anyone using them has to import them explicitly. So I suggest that they are converted to be member functions (which is trivial to do).

Secondly, the use of the python math functions sqrt and floor is not a good idea, since the coefficients of the form will be Sage integers. For example:

```sage: from sage.quadratic_forms.binary_qf import normalized_indefinite_form
sage: f = BinaryQF([10^100,10^500,10^200])
sage: f.discriminant()>0
True
sage: normalized_indefinite_form(f)
---------------------------------------------------------------------------
OverflowError                             Traceback (most recent call last)

/home/john/.sage/temp/ubuntu/9768/_home_john__sage_init_sage_0.py in <module>()

486         raise ValueError, "This only works for indefinite forms"
487
--> 488     if sqrt(delta) <= abs(a):
489         s=(a/abs(a))*floor(-1*b/(2*abs(a)))
490     else:

OverflowError: math range error
```

For example, the line if sqrt(delta) <= abs(a): could be replaced by if delta <= a2: .

The docstring for this function should also say what the definition of "normalised" is.

I also slightly amended the ticket's title to give a better description.