# Ticket #12860(closed defect: fixed)

Opened 14 months ago

## Incorrect computation of maximal orders in quaternion algebras

Reported by: Owned by: daniels daniels major sage-5.3 algebra N/A Aly Deines Daniel Smertnig sage-5.3.beta0

The maximal_order function of QuaternionOrder? can produce incorrect results. It is implemented by means of an explicit formula, which expects not just the algebra to be ramified precisely at one finite prime p and infinity, but also expect the invariants a=i2 and b=j2 to have a special form. Not all the necessary conditions are checked by the code.

E.g. the following is obviously not an order since it contains non-integral elements:

```sage: A.<i,j,k> = QuaternionAlgebra(17)
sage: print A.invariants()
(-3, -17)
sage: R = A.maximal_order()
sage: b = R.basis()
sage: print b
(1/2 + 1/2*j, 1/2*i + 1/2*k, -1/3*j - 1/3*k, k)
sage: b[0]*b[1]
9/2*i
sage: (b[0]*b[1]).reduced_norm()
243/4
```

But this is ok, because now the invariants are as in Pizer's paper:

```sage: A.<i,j,k> = QuaternionAlgebra(-17,-3)
sage: print A.maximal_order().basis()
(1/2 + 1/2*j, 1/2*i + 1/2*k, -1/3*j - 1/3*k, k)
```

This could be fixed by simply swapping (i,j) in the formula depending on the invariants, but the invariants can deviate from the required form in more than one way, again possibly resulting in failure of the formula:

```sage: A.<i,j,k> = QuaternionAlgebra(-17*9,-3)
sage: print A.maximal_order().basis()
(1/2 + 1/2*j, 1/2*i + 1/2*k, -1/3*j - 1/3*k, k)
sage: print (-1/3*j - 1/3*k).reduced_norm()
154/3
```

(#11464 is related)

## Change History

### comment:1 Changed 14 months ago by daniels

• Description modified (diff)

The patch does the following:

• Move maximal_order from brandt.py to quaternion_algebra.py.
• Add checks in ''__init__'' of QuaternionOrder? to verify if the given basis actually is the basis of an order (this would have catched the bug earlier).
• Fix all the doctests that were expecting the mathematically incorrect results.

### comment:2 Changed 14 months ago by daniels

See also #12861, for a patch extending this one by a more general algorithm.

### comment:3 Changed 14 months ago by daniels

New patch applies to sage-5.0.beta13 (old one conflicted with #12461)

### comment:4 Changed 11 months ago by aly.deines

• Status changed from new to needs_review

This applies to sage-5.2.rc0 and all tests pass.

This looks good to me!

### comment:5 Changed 11 months ago by aly.deines

• Status changed from needs_review to positive_review

### comment:8 Changed 11 months ago by aly.deines

• Reviewers set to Aly Deines

### comment:9 Changed 11 months ago by daniels

• Authors set to Daniel Smertnig

### comment:10 Changed 11 months ago by jdemeyer

• Status changed from positive_review to closed
• Resolution set to fixed
• Merged in set to sage-5.3.beta0
Note: See TracTickets for help on using tickets.