Opened 8 years ago
Closed 7 years ago
#16055 closed enhancement (fixed)
Implement Jordan algebras
Reported by:  tscrim  Owned by:  tscrim 

Priority:  major  Milestone:  sage6.4 
Component:  algebra  Keywords:  jordan algebra 
Cc:  Merged in:  
Authors:  Travis Scrimshaw  Reviewers:  Darij Grinberg 
Report Upstream:  N/A  Work issues:  
Branch:  10b1863 (Commits, GitHub, GitLab)  Commit:  10b1863c7403f29658914fe923c87ae01bae0225 
Dependencies:  Stopgaps: 
Description (last modified by )
So we have a nonassociative algebra to play with (and they are connected with Lie algebras).
This currently uses a workaround for avoiding issues with unital nonassociative algebras, see #16492.
Change History (32)
comment:1 Changed 8 years ago by
 Branch set to public/algebras/jordan16055
 Commit set to 7e002a7580e2de0f9bff620e652234bdd2893e2a
 Status changed from new to needs_review
comment:2 Changed 8 years ago by
 Commit changed from 7e002a7580e2de0f9bff620e652234bdd2893e2a to a0111313bb850bf7bf9e8a1d3c6cf5138ae7b23f
Branch pushed to git repo; I updated commit sha1. New commits:
a011131  Added reference.

comment:3 Changed 8 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:4 Changed 8 years ago by
 Commit changed from a0111313bb850bf7bf9e8a1d3c6cf5138ae7b23f to cd1248bf099afc4b6cfa5ce940c51bcc2a2d0ef9
Branch pushed to git repo; I updated commit sha1. New commits:
4c4c2a2  Merge branch 'public/algebras/jordan16055' of trac.sagemath.org:sage into public/algebras/jordan16055

da9d39c  Changed to use MagmaticAlgebras.

cd1248b  Better generator support for special Jordan algebras and workaround for #16492.

comment:5 Changed 8 years ago by
 Description modified (diff)
comment:6 Changed 7 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:7 Changed 7 years ago by
 Commit changed from cd1248bf099afc4b6cfa5ce940c51bcc2a2d0ef9 to 6e76ec0b3df76d3ba3bf326b851f494e71bc58bd
comment:8 Changed 7 years ago by
 Commit changed from 6e76ec0b3df76d3ba3bf326b851f494e71bc58bd to c52895d3980b7c8d23bc7d8a5eda9f7622bd300c
comment:9 followup: ↓ 10 Changed 7 years ago by
Thank you! Just to make sure, _lmul_
only gets called when you multiply with a scalar?
EDIT: Also, I don't see the (x^m y) x^n = x^m (y x^n)
identity in McCrimmon?...
comment:10 in reply to: ↑ 9 Changed 7 years ago by
Replying to darij:
Thank you! Just to make sure,
_lmul_
only gets called when you multiply with a scalar?'s
Correct. If it's an element of the Jordan algebra, then regular _mul_
gets called.
EDIT: Also, I don't see the
(x^m y) x^n = x^m (y x^n)
identity in McCrimmon?...
It's not. I cited Jacobson (as I believe it's in there but I don't have the book right now to check [tonight I will]), but it's on the wikipedia page (which I didn't want to explicitly cite).
comment:11 Changed 7 years ago by
Found it: equality (11) in
A. A. Albert, *A Structure Theory for Jordan Algebras*, Annals of Mathematics, Second Series, Vol. 48, No. 3 (Jul., 1947), pp. 546567.
I'll put this into the doc later.
comment:12 Changed 7 years ago by
It's not in Jacobson; I'm glad you found an explicit reference.
comment:13 Changed 7 years ago by
 Commit changed from c52895d3980b7c8d23bc7d8a5eda9f7622bd300c to b6480617ed2b3902edcf1497c1c9518729c40445
comment:14 Changed 7 years ago by
 Commit changed from b6480617ed2b3902edcf1497c1c9518729c40445 to 021551c8d2a0d6f9d6751176a8c1fe65302ff2a0
Branch pushed to git repo; I updated commit sha1. New commits:
021551c  oops

comment:15 Changed 7 years ago by
Here's why the ~
is safer than /
:
sage: R.<q,t> = LaurentPolynomialRing(QQ) sage: 1 / R(2) 1/2 sage: _.parent() Fraction Field of Multivariate Polynomial Ring in q, t over Rational Field sage: ~R(2) 1/2 sage: _.parent() Multivariate Laurent Polynomial Ring in q, t over Rational Field
Although this might be fixed by #17740. However I think instead of QQ
, we could just convert to (the fraction field of) the base ring. I'll test and make changes.
I'm happy with setting algebras_generators = basis
, and subclassing does not respect aliases (this has given me many headaches, but it makes sense from the python perspective). I'll do this on my next commit.
comment:16 Changed 7 years ago by
 Commit changed from 021551c8d2a0d6f9d6751176a8c1fe65302ff2a0 to 5dd9543f4c008197d78cb523f18418873c5c8ef0
Branch pushed to git repo; I updated commit sha1. New commits:
5dd9543  Fixes and tweaks based on Darij's comments.

comment:17 Changed 7 years ago by
Thanks for the explanations. I was worried that subclassing wouldn't completely ignore aliases in case of @cached_function usage. But experiments suggest that there is no problem with that.
sage: J(2, (2, 3)) # known bug  defaults to the base ring morphism #16054 2 + (2, 3) sage: J(1, 1, 0) # known bug  defaults to the base ring morphism #16054 1 + (1, 0)
But these work!
comment:18 Changed 7 years ago by
 Commit changed from 5dd9543f4c008197d78cb523f18418873c5c8ef0 to ec86f6a3628c1dae0769dc38991735df5b5aa0c1
Branch pushed to git repo; I updated commit sha1. New commits:
ec86f6a  Changes to Sage have made other inputs valid.

comment:19 followup: ↓ 21 Changed 7 years ago by
I'm glad you tested that. That used to not work, but now since there is no coercion map from the base ring (when I first wrote it, there was one), it doesn't go through the coercion path noted in #16054.
comment:20 Changed 7 years ago by
 Commit changed from ec86f6a3628c1dae0769dc38991735df5b5aa0c1 to 7ed54709f4644efea728050b8b7ea64cc5fd7576
Branch pushed to git repo; I updated commit sha1. New commits:
7ed5470  complete review

comment:21 in reply to: ↑ 19 ; followup: ↓ 23 Changed 7 years ago by
Replying to tscrim:
I'm glad you tested that. That used to not work, but now since there is no coercion map from the base ring (when I first wrote it, there was one), it doesn't go through the coercion path noted in #16054.
Does this mean that this will break again when the base ring coercion is reintroduced?
Also, I've finished reviewing this, but there is a TODO in the latest commit.
comment:22 Changed 7 years ago by
 Commit changed from 7ed54709f4644efea728050b8b7ea64cc5fd7576 to 4404242dbb01346805cbdce258053636356f71b6
Branch pushed to git repo; I updated commit sha1. New commits:
4404242  Fixing sign errors.

comment:23 in reply to: ↑ 21 Changed 7 years ago by
 Reviewers set to Darij Grinberg
Replying to darij:
Does this mean that this will break again when the base ring coercion is reintroduced?
This isn't a coercion for noncommutative base rings. The commutativity of the associative algebra actually suppresses the nonassociativity of the Jordan algebra since
(xy + yx)/2 = xy = yx (yz + zy)/2 = yz = zy (xyz + zyx)/2 = xyz = yxz = ... = zyx
Perhaps we could add extra checks for the base ring being commutative and adding such a coercion if there is a need, but for now I think we have the conversion and left/right actions.
Also, I've finished reviewing this, but there is a TODO in the latest commit.
Fixed; sign error. So can I consider this as a positive review?
comment:24 Changed 7 years ago by
Uhm, the base ring is assumed to be commutative, right? Are you talking about a coercion from the base ring, or from the algebra which serves as a ground set? Which of these would break the element constructor?
And yes, this is a positive review, apart from this little issue.
comment:25 Changed 7 years ago by
Right, we are... The issue is when we have a coercion from the base ring, the first input gets checked to see if there is a coercion, and if there is, then it passes all arguments via that coercion (ignoring that the combination of elements should give a different coercion/conversion). So I could one of the two:
 leave it as it is,
 add a custom coercion from the base ring and put back the known bug tag.
Which is your preference?
comment:26 Changed 7 years ago by
I'd leave it as it is  it does make sense that a map from an associative into a nonassociative ring is only a conversion. Can you add in a comment on this?
comment:27 Changed 7 years ago by
On that those doctests will break if there is a coercion?
comment:28 Changed 7 years ago by
Well, you currently have this line:
+ # Remove the preceding line once :trac:`16492` is fixed
You should say that #16492 is not the only obstruction to the coercion.
Once done, this is positive_review! I'd make the change myself, but that would add yet another merge commit.
comment:29 Changed 7 years ago by
 Commit changed from 4404242dbb01346805cbdce258053636356f71b6 to 10b1863c7403f29658914fe923c87ae01bae0225
Branch pushed to git repo; I updated commit sha1. New commits:
10b1863  Expanded a comment.

comment:30 Changed 7 years ago by
 Status changed from needs_review to positive_review
Done. Thanks for doing the review Darij!
comment:31 Changed 7 years ago by
No problem  this is probably the easiest of your Liealgebra tickets.
comment:32 Changed 7 years ago by
 Branch changed from public/algebras/jordan16055 to 10b1863c7403f29658914fe923c87ae01bae0225
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Skeleton code for Jordan algebras.
Added Jordan algebra from a symmetric bilinear form.