Add test if quaternion order is maximal #37111
Conversation
… fields Via discriminant comparison
| @@ -1758,9 +1759,48 @@ def discriminant(self): | |||
|
|
|||
| return (MatrixSpace(QQ, 4, 4)(L)).determinant().sqrt() | |||
|
|
|||
| def is_maximal(self): | |||
| r""" | |||
| Return ``True`` if the order is maximal in the ambient quaternion algebra, ``False`` otherwise | |||
There was a problem hiding this comment.
The first line should be short and usually we start such description with Check whether...
- Return ``True`` if the order is maximal in the ambient quaternion algebra, ``False`` otherwise
- Only works in quaternion algebras over number fields
+ Check whether the order of ``self`` is maximal in the ambient
+ quaternion algebra.
+
+ Only works in quaternion algebras over number fields| ....: print("This should fail") | ||
| ....: except NotImplementedError: | ||
| ....: print("ok") | ||
| ....: |
There was a problem hiding this comment.
Without object now since the test was completely rewritten following @pjbruin's remarks.
| ....: | ||
| ok | ||
| """ | ||
| if not(self.quaternion_algebra().base_ring() in NumberFields()): |
There was a problem hiding this comment.
It is better to use "X not in Y". Furthermore, we don't start error messages with capital letters and the dot at the end is not needed.
- if not(self.quaternion_algebra().base_ring() in NumberFields()):
- raise NotImplementedError("Check for maximality is only implemented for quaternion algebras over number fields).")
- return(self.discriminant() == self.quaternion_algebra().discriminant())
+ if self.quaternion_algebra().base_ring() not in NumberFields():
+ raise NotImplementedError("check for maximality is only implemented for "
+ "quaternion algebras over number fields")
+ return self.discriminant() == self.quaternion_algebra().discriminant()
| def left_ideal(self, gens, check=True, *, is_basis=False): | ||
| r""" | ||
| Return the left ideal of this order generated by the given generators. | ||
| Only works in quaternion algebras over number fields. |
There was a problem hiding this comment.
Please add an empty line after the first sentence of the docstring (see dcoudert's comment above).
| sage: i,j,k = B.gens() | ||
| sage: O0_basis = (1,i,j, k) | ||
| sage: O0 = B.quaternion_order(O0_basis) | ||
| sage: try: |
There was a problem hiding this comment.
The recommended way to write doctests for error messages is
sage: O0.is_maximal()
Traceback (most recent call last):
...
NotImplementedError: check for maximality is only implemented for quaternion algebras over number fields
| sage: p = 11 | ||
| sage: B = QuaternionAlgebra(QQ,-1,-p) | ||
| sage: i,j,k = B.gens() | ||
| sage: O0_basis = (1,i,(i+j)/2, (1+i*j)/2) |
There was a problem hiding this comment.
fix spacing (after each comma there should be a space except in some specific cases)
There was a problem hiding this comment.
I tried, hopefully I did not miss any comma and never met such a specific case.
There was a problem hiding this comment.
The main example is in one-element tuples, such as (1,). I also tend to omit spaces when declaring multiple symbolic variables (var('x,y')), but I'm not sure if this is generally accepted.
Also remove accidental change to documentation of unrelated function from previous commit
|
I hopefully followed all of your suggestions. Thank you for the explanations, and please tell me if I missed something. |
|
Documentation preview for this PR (built with commit c603b36; changes) is ready! 🎉 |
Add a method to sage.algebras.quatalg.quaternion_algebra.QuaternionOrder which allows to check whether the order is maximal.
This method compares the discriminant of the order to the discriminant of the algebra. According to Voight's book on quaternion algebras (chapter 15), this test is valid in number fields (but not over any field), so the method fails with a notImplementedError in other cases.
Currently, there is no method to test maximality of orders, so implementing it for quaternion algebras over number fields is some progress.
#sd123