Skip to content
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

Quaternion maximal_order fails #37417

Closed
2 tasks done
grhkm21 opened this issue Feb 21, 2024 · 1 comment
Closed
2 tasks done

Quaternion maximal_order fails #37417

grhkm21 opened this issue Feb 21, 2024 · 1 comment

Comments

@grhkm21
Copy link
Contributor

grhkm21 commented Feb 21, 2024

Steps To Reproduce

B = QuaternionAlgebra(-4, -28)
B.maximal_order()

Expected Behavior

work

Actual Behavior

not

Additional Information

Not sure if this is normal, as I don't have enough quaternion knowledge to determine.

@yyyyx4

Environment

N/A

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
@grhkm21 grhkm21 changed the title maximal_order fails Quaternion maximal_order fails Feb 21, 2024
@grhkm21
Copy link
Contributor Author

grhkm21 commented Feb 21, 2024

Sorry, I missed #37217 .

@grhkm21 grhkm21 closed this as completed Feb 21, 2024
GiacomoPope added a commit to GiacomoPope/sage that referenced this issue Feb 25, 2024
S17A05 added a commit to S17A05/sage_development that referenced this issue Mar 21, 2024
S17A05 added a commit to S17A05/sage_development that referenced this issue Mar 21, 2024
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 11, 2024
…l_order()` of `quaternion_algebra.py`

    
- Corrected in `.normalize_basis_at_p` the wrong assignments of `f0,
f1`: The idea is to replace `f0` by `f0 + f1` and `f1` by `f0`
_simultaneously_ - the original code, however, first replaced `f0` by
`f0 + f1` and then copied the value to `f1`, hence causing a decrease in
dimension
- Corrected an index error just before the recursive call of
`normalize_basis_at_p` in the off-diagnonal case for `p = 2` (the
`else`-block)
- In `.maximal_order()` the intermediate basis `e_n` might not define an
order anymore (as seen in the comments, this was known to the author(s)
of the method) - therefore we need to manually compute the discriminant
in the loop instead of relying on the `.discriminant()`-method of
quaternion orders
- Adapted an example in `.maximal_order()` to use the new method
`.is_maximal()`

Fixes sagemath#37217 (both parts) and sagemath#37417.

Disclaimer:
I am aware of sagemath#37239, but since progress on that PR seems to have
halted, I took the liberty to fix the issues myself :)
    
URL: sagemath#37644
Reported by: Sebastian A. Spindler
Reviewer(s): Matthias Köppe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant