Skip to content

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

Merged
merged 11 commits into from
Apr 27, 2024

Conversation

S17A05
Copy link
Member

@S17A05 S17A05 commented Mar 26, 2024

  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 #37644: Naturally, the extended algorithm can only work correctly if the base algorithm works correctly in the first place.

S17A05 added 2 commits April 6, 2024 18:29
Caused by switched order of input arguments

Amend: This time for real
@S17A05 S17A05 force-pushed the maximal_extension branch from c9885e1 to c29ee5c Compare April 6, 2024 18:52
Copy link

github-actions bot commented Apr 6, 2024

Documentation preview for this PR (built with commit bf39355; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@S17A05
Copy link
Member Author

S17A05 commented Apr 13, 2024

The testing timeout of algebras.fusion_rings.fusion_ring in the CI should be unrelated; I can't reproduce it locally, though.

Credit to @tscrim for the help on how to do this!
Copy link
Collaborator

@tscrim tscrim left a 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
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 20, 2024
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
@vbraun vbraun merged commit 1cd71c6 into sagemath:develop Apr 27, 2024
14 checks passed
@mkoeppe mkoeppe added this to the sage-10.4 milestone Apr 27, 2024
@S17A05 S17A05 deleted the maximal_extension branch April 27, 2024 18:04
S17A05 added a commit to Eloitor/sagemath that referenced this pull request Feb 21, 2025
- 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants