-
-
Notifications
You must be signed in to change notification settings - Fork 598
Allow .maximal_order()
of quaternion_algebra.py
to extend a given order
#37675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
... and small refactoring of code
Caused by switched order of input arguments Amend: This time for real
c9885e1
to
c29ee5c
Compare
Documentation preview for this PR (built with commit bf39355; changes) is ready! 🎉 |
The testing timeout of |
Credit to @tscrim for the help on how to do this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Two trivial things that I would appreciate. Once done (or rejected), then you can set a positive review.
- Corrected order of additional arguments - Fixed spacing for PEP8
sagemathgh-37675: Allow `.maximal_order()` of `quaternion_algebra.py` to extend a given order 1. Added an optional argument `order_basis` to `.maximal_order()` to allow for extension of orders: If `order_basis` defines an order of the given quaternion algebra, then the method now returns a maximal order that contains the order specified by the given basis. ~~2. Added comparison functions `__le__`, `__ge__`, `__lt__` and `__gt__` for quaternion orders.~~ ~~3. Small edits of the comparison functions `__eq__` and `__ne__` for quaternion orders.~~ 2. Added a general comparison function `__richcmp__` for quaternion orders and removed the explicit implementation of `__eq__` and `__ne__`. More details on 1.: The algorithm used in `.maximal_order()` currently uses the standard basis `(1,i,j,k)` as a starting basis, but it extends to any starting basis whose first entry is equal to `1`. Using the helper method `basis_for_quaternion_lattice`, we transform any quaternion order basis input by the user to a basis of this form. Depends on sagemath#37644: Naturally, the extended algorithm can only work correctly if the base algorithm works correctly in the first place. URL: sagemath#37675 Reported by: Sebastian A. Spindler Reviewer(s): Sebastian A. Spindler, Travis Scrimshaw
- Added missing tests for possible errors - Integrated `.ramified_places()` to check that new construction works correctly - Modified input checks to properly reject bad inputs - Added author entry for prior work on sagemath#37173, sagemath#37644 and sagemath#37675
order_basis
to.maximal_order()
to allow for extension of orders: Iforder_basis
defines an order of the given quaternion algebra, then the method now returns a maximal order that contains the order specified by the given basis.2. Added comparison functions__le__
,__ge__
,__lt__
and__gt__
for quaternion orders.3. Small edits of the comparison functions__eq__
and__ne__
for quaternion orders.__richcmp__
for quaternion orders and removed the explicit implementation of__eq__
and__ne__
.More details on 1.:
The algorithm used in
.maximal_order()
currently uses the standard basis(1,i,j,k)
as a starting basis, but it extends to any starting basis whose first entry is equal to1
. Using the helper methodbasis_for_quaternion_lattice
, we transform any quaternion order basis input by the user to a basis of this form.Depends on #37644: Naturally, the extended algorithm can only work correctly if the base algorithm works correctly in the first place.