sagemath / sage

Main repository of SageMath. Now open for Issues and Pull Requests.
https://www.sagemath.org
Other
1.2k stars 413 forks source link

Quaternion Orders over Number Fields #13568

Open 4dfe6752-6233-4f7f-8e0e-e1363b12c1b7 opened 11 years ago

4dfe6752-6233-4f7f-8e0e-e1363b12c1b7 commented 11 years ago

Generalizes the code for quaternion orders of quaternion algebras over QQ to allow for quaternion orders of quaternion algebras over number fields.

CC: @sagetrac-schisholm @dansme

Component: algebra

Keywords: quaternion, order

Author: Aly Deines

Branch/Commit: u/aurel/13568-review @ afba274

Reviewer: Gonzalo Tornaría

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

4dfe6752-6233-4f7f-8e0e-e1363b12c1b7 commented 11 years ago

Attachment: Trac13569_quaternion_orders.patch.gz

tornaria commented 9 years ago
comment:8

Rebased the patch to sage-6.4.beta1 and moved to git

tornaria commented 9 years ago

Commit: 733e651

tornaria commented 9 years ago

Branch: u/tornaria/ticket/13568

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 733e651 to 9858d7e

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

e495dc1Trac #13569: fix whitespace
70614ddTrac #13569: fix usage of raise
0a3ca50Trac #13569: remove unnecessary import
4fbac85Trac #13569: style changes
f824306Trac #13569: Revert basis of quaternion algebras and orders to be tuples
9858d7eTrac #13569: document and test new argument ideal_list
tornaria commented 9 years ago
comment:11

I've pushed a bunch of reviewer commits, the log explains them (esp f824306).

There's still a bug, namely when the base field is QQ and a pseudo-basis is given, the current code will just ignore the ideals.

4dfe6752-6233-4f7f-8e0e-e1363b12c1b7 commented 9 years ago
comment:12

Okay. I'll get on that.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 9858d7e to 1e8d1e2

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

1e8d1e2Trac #13569: fix when base_field=QQ and pseudo-basis is given
tornaria commented 9 years ago
comment:14

I'm happy with this now. Since I made a lot of changes I'm leaving it as needs_review.

tornaria commented 9 years ago

Reviewer: tornaria

4dfe6752-6233-4f7f-8e0e-e1363b12c1b7 commented 9 years ago

Changed branch from u/tornaria/ticket/13568 to u/aly.deines/ticket/13568

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 1e8d1e2 to bd9fe85

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

bd9fe85Some cleanup.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from bd9fe85 to 9f9af0d

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

9f9af0dFixed the Brandt.py documentation.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

130d486Removed extra generated files.
8ed696dDelated an extra git-trac-command folder.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from 9f9af0d to 8ed696d

mmasdeu commented 9 years ago

Changed commit from 8ed696d to 73909fd

mmasdeu commented 9 years ago

Changed branch from u/aly.deines/ticket/13568 to u/mmasdeu/13568-review

mmasdeu commented 9 years ago
comment:20

I started reviewing it, and made some changes. Some are to make it more pythonic, some are for clarity.

I've set it to needs_work because the patch fails when using quaternion algebras not defined over number fields:

sage: A.<i,j,k> = QuaternionAlgebra(ZZ['x'].fraction_field(),-1,-1)
sage: A.quaternion_order([1,i,j,k])                                
---------------------------------------------------------------------------
UnboundLocalError                         Traceback (most recent call last)
...
UnboundLocalError: local variable 'M1' referenced before assignment
jdemeyer commented 9 years ago
comment:21

Just a remark: this functionality is in PARI now, so you could just try to wrap PARI.

cd1fd1a3-1fd5-4c80-88ce-04d187901b3d commented 7 years ago

Changed branch from u/mmasdeu/13568-review to u/aurel/13568-review

cd1fd1a3-1fd5-4c80-88ce-04d187901b3d commented 7 years ago

Changed commit from 73909fd to afba274

cd1fd1a3-1fd5-4c80-88ce-04d187901b3d commented 7 years ago
comment:23

With Marc Masdeu, we checked that this is still compiling after rebasing on develop. We intend to have a look at this ticket.


Last 10 new commits:

c4276e6Fixed a merging error.
055e633Changed to tuples.
18c5420More fixes.
5b1046eSome cleanup.
9c67df8Fixed the Brandt.py documentation.
188229bRemoved extra generated files.
f668c09Delated an extra git-trac-command folder.
b7a7864Trac 13568: Reviewer's patch.
aa554bcFixed transpose mistake.
afba274Some minor changes, initialized __basis and __pseudobasis to None.
jdemeyer commented 7 years ago

Changed dependencies from #13509 - Pari nfhnf to none

jdemeyer commented 7 years ago

Changed reviewer from tornaria to Gonzalo Tornaría