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

every quaternion lies in every quaternion order #32364

Closed
yyyyx4 opened this issue Aug 11, 2021 · 21 comments
Closed

every quaternion lies in every quaternion order #32364

yyyyx4 opened this issue Aug 11, 2021 · 21 comments

Comments

@yyyyx4
Copy link
Member

yyyyx4 commented Aug 11, 2021

This is wrong:

sage: Q.<i,j,k> = QuaternionAlgebra(-1,-19)
sage: O = Q.quaternion_order([1,i,j,k])
sage: 1/5 in O
True
sage: (i+j)/123 in O
True

To fix this, we add an _element_constructor_ to QuaternionOrders that actually checks whether the given element is contained in the order. (This in turn makes the default implementation of __contains__ work properly.)

CC: @pjbruin

Component: algebra

Keywords: quaternion orders, elements, membership

Stopgaps: mathematically_wrong

Author: Lorenz Panny

Branch/Commit: a1d445b

Reviewer: Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/32364

@yyyyx4 yyyyx4 added this to the sage-9.4 milestone Aug 11, 2021
@yyyyx4
Copy link
Member Author

yyyyx4 commented Aug 11, 2021

Author: Lorenz Panny

@yyyyx4
Copy link
Member Author

yyyyx4 commented Aug 11, 2021

@yyyyx4
Copy link
Member Author

yyyyx4 commented Aug 11, 2021

Commit: e647fad

@yyyyx4

This comment has been minimized.

@fchapoton
Copy link
Contributor

comment:2

Here

+        TESTS::
+
+        Test for :trac:`32364`:

there should be a single colon on top line and a double colon on bottom lines. Double colons are signaling an indented block.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 11, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

81b93c1fix doctest formatting

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 11, 2021

Changed commit from e647fad to 81b93c1

@yyyyx4
Copy link
Member Author

yyyyx4 commented Aug 11, 2021

comment:4

Replying to @fchapoton:

there should be a single colon on top line and a double colon on bottom lines. Double colons are signaling an indented block.

Oops! Thanks, fixed.

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 22, 2021
@yyyyx4
Copy link
Member Author

yyyyx4 commented Aug 26, 2021

Stopgaps: mathematically_wrong

@yyyyx4
Copy link
Member Author

yyyyx4 commented Oct 21, 2021

comment:7

Bumping priority since this bug can lead to mathematical errors.

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 18, 2021

comment:8

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.

@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@slel
Copy link
Member

slel commented Mar 22, 2022

comment:9

Reported again at

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2022

Changed commit from 81b93c1 to ff7ad9d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

ff7ad9dMerge tag '9.6.beta5' into public/fix_quaternion_order_element_constructor

@fchapoton
Copy link
Contributor

comment:11

please break the very long line in doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2022

Changed commit from ff7ad9d to a1d445b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

a1d445bbreak long line

@yyyyx4
Copy link
Member Author

yyyyx4 commented Mar 23, 2022

comment:13

Done & patchbot is green.

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:14

ok, then

@vbraun
Copy link
Member

vbraun commented Mar 30, 2022

Changed branch from public/fix_quaternion_order_element_constructor to a1d445b

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

5 participants