Opened 7 years ago
Closed 5 years ago
#16820 closed enhancement (fixed)
Implement ABCs and categories for Lie algebras and finite dimensional Lie algebras given by structure coefficients
Reported by:  tscrim  Owned by:  tscrim 

Priority:  major  Milestone:  sage8.0 
Component:  algebra  Keywords:  lie algebras, days64, sd67, days74, days79 
Cc:  Merged in:  
Authors:  Travis Scrimshaw  Reviewers:  Darij Grinberg, Eric Gourgoulhon 
Report Upstream:  N/A  Work issues:  
Branch:  98c488d (Commits, GitHub, GitLab)  Commit:  98c488db05e40d8fd099673e8f8274febc2a8657 
Dependencies:  Stopgaps: 
Description (last modified by )
Part of #14901. This implements the basic hierarchy of base classes, the categories for Lie algebras, some standard Lie algebras, and finitedimensional Lie algebras given by structure coefficients.
Change History (146)
comment:1 Changed 7 years ago by
 Branch set to public/lie_algebras/fd_structure_coeff16820
 Commit set to c2a3a982e419c0839f1fba68bf71b1014f243547
 Status changed from new to needs_review
comment:2 Changed 7 years ago by
 Commit changed from c2a3a982e419c0839f1fba68bf71b1014f243547 to e5c1703bc6c958566a5f2a91d909e45b52b3c2b0
Branch pushed to git repo; I updated commit sha1. New commits:
c1f3eb3  Merge branch 'develop' into public/lie_algebras/fd_structure_coeff16820

d0ddb8c  Merge branch 'develop' into public/lie_algebras/fd_structure_coeff16820

7424e75  Merge branch 'develop' into public/lie_algebras/fd_structure_coeff16820

e5c1703  Fixed typos with sl2 construction.

comment:3 Changed 7 years ago by
 Commit changed from e5c1703bc6c958566a5f2a91d909e45b52b3c2b0 to 87bba39ede0cfea37bb5e92b81aa851a2420863e
Branch pushed to git repo; I updated commit sha1. New commits:
45db323  Merge branch 'develop' into public/lie_algebras/fd_structure_coeff16820

036995f  Cleaning up/Fixing some logic, in particular for FromAssociative.

a135fb3  Merge branch 'develop' into public/lie_algebras/fd_structure_coeff16820

b4b89da  Fixing errors introduced by logic changes.

e9587d7  Added a custom lift morphism for Lie alg from assoc.

87bba39  Fixing the last issues from the changes.

comment:4 Changed 7 years ago by
 Commit changed from 87bba39ede0cfea37bb5e92b81aa851a2420863e to ece3cd9d8ae826f28b82935d638de1f5df9d90a9
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
65c1f54  Removed the LiftMorphism.section map.

becabaa  Merge branch 'public/categories/lie_algebras16819' of trac.sagemath.org:sage into public/categories/lie_algebras16819

980545e  Fixed the doc.

8a6950a  Merge branch 'public/categories/lie_algebras16819' of git://trac.sagemath.org/sage into lie1

9b08311  a few questions to boot

5a0bc7d  Answering some questions.

144a5a5  Small fix for the doc.

198f8a8  Added new categories to documentation.

aeb6075  Merge branch 'public/categories/lie_algebras16819' into public/lie_algebras/fd_structure_coeff16820

ece3cd9  Added files to the documentation.

comment:5 Changed 7 years ago by
 Commit changed from ece3cd9d8ae826f28b82935d638de1f5df9d90a9 to 845e5aa14b7c97edeb04859fc92ebc63a78be1cb
comment:6 Changed 7 years ago by
 Keywords lie algebras days64 added
 Milestone changed from sage6.4 to sage6.6
comment:7 Changed 7 years ago by
 Commit changed from 845e5aa14b7c97edeb04859fc92ebc63a78be1cb to 12074e86295e2848a520c5962678c1c9e75b7505
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
896028a  A y for an x.

2d2a54d  Merge branch 'public/categories/lie_algebras16819' into public/lie_algebras/fd_structure_coeff16820

6ca7aca  Fixing some parts of _construct_UEA.

d35b756  Adding a default _basis_ordering and _dense_free_module

0e33276  Merge branch 'public/categories/lie_algebras16819' into public/lie_algebras/fd_structure_coeff16820

e95d67c  More fixes to findim w/ basis category.

c800d6c  Merge branch 'public/categories/lie_algebras16819' into public/lie_algebras/fd_structure_coeff16820

a491bb0  Some more cleanup and fixes.

aabf540  Fixing coefficient division.

12074e8  Merge branch 'public/categories/lie_algebras16819' into public/lie_algebras/fd_structure_coeff16820

comment:8 Changed 7 years ago by
 Commit changed from 12074e86295e2848a520c5962678c1c9e75b7505 to 9830d63d19d1c4ec432a83ab8c21685ec7aae802
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
464c131  another appearance of free_module

6608a18  .module and .to_vector: I still don't get them

106d75d  Merge branch 'public/categories/lie_algebras16819' of git://trac.sagemath.org/sage into lie1

6201720  does this make sense?

ddc77b1  Merge branch 'public/categories/lie_algebras16819' of git://trac.sagemath.org/sage into lie1a

b0e36fe  changes I forgot to commit a few weeks ago

b412977  manual merge with 6.7.beta0

de60849  I have no idea what I'm doing

0c28f5f  reverting parts of previous commit with fire

9830d63  Merge branch 'public/lie_algebras/fd_structure_coeff16820' of git://trac.sagemath.org/sage into lie2

comment:9 Changed 7 years ago by
 Commit changed from 9830d63d19d1c4ec432a83ab8c21685ec7aae802 to 3d450840695003e8e410e0d0e3b926d7a00abe23
comment:10 Changed 7 years ago by
Just had a quick look at some of the examples. Please check my last commit and revert if necessary (I cannot test right now since Sage is compiling again).
comment:11 Changed 7 years ago by
 Commit changed from 3d450840695003e8e410e0d0e3b926d7a00abe23 to 5e9a839618ae034d04c13340603a8b62d4bbf2f8
Branch pushed to git repo; I updated commit sha1. New commits:
d952a7b  Merge branch 'public/categories/lie_algebras16819' into 6.7.b1

dfad36c  trac #16819 wrong inclusion into doc is corrected

eb3b058  hopefully fix doc bug

498af17  Merge branch 'public/lie_algebras/fd_structure_coeff16820' of git://trac.sagemath.org/sage into lie2

5e9a839  Jacobi identity now tested

comment:12 Changed 7 years ago by
 Commit changed from 5e9a839618ae034d04c13340603a8b62d4bbf2f8 to 005984dfd1e329136db896a7dd29c1a2153dcff6
Branch pushed to git repo; I updated commit sha1. New commits:
005984d  document the broken multiplication by base ring

comment:13 Changed 7 years ago by
 Commit changed from 005984dfd1e329136db896a7dd29c1a2153dcff6 to 5bf7cf7b377985c6374e1a907f517cae62c063ea
Branch pushed to git repo; I updated commit sha1. New commits:
5bf7cf7  documenting LieAlgebraWithStructureCoefficients (mostly scaffolding for myself)

comment:14 Changed 7 years ago by
 Commit changed from 5bf7cf7b377985c6374e1a907f517cae62c063ea to 4e8469da080ac7d303b74a8c697c6687b28e4dc3
Branch pushed to git repo; I updated commit sha1. New commits:
4e8469d  fixing own bugs, and heeding pyflakes

comment:15 Changed 7 years ago by
 Commit changed from 4e8469da080ac7d303b74a8c697c6687b28e4dc3 to 878ab1d0ed5dc2b0e27f1257caccb3747009bfb1
Branch pushed to git repo; I updated commit sha1. New commits:
878ab1d  make generators work correctly for rank0 Heisenberg algebra

comment:16 Changed 7 years ago by
 Commit changed from 878ab1d0ed5dc2b0e27f1257caccb3747009bfb1 to 6f6c529b753631bde338b7225968619e193db794
Branch pushed to git repo; I updated commit sha1. New commits:
6f6c529  pWitt algebras, so that we have a characteristicp example

comment:17 Changed 7 years ago by
 Commit changed from 6f6c529b753631bde338b7225968619e193db794 to 052721a72568247cafc5abff310ad3478f999961
Branch pushed to git repo; I updated commit sha1. New commits:
052721a  I don't know why (and in what sense) three_dimensional covers "all 3dim Lie algebras", so I weakened that claim. (Maybe it does over fields, but I somehow doubt that it works in general.) Also, corrected a bordercase failure for empty gens.

comment:18 Changed 7 years ago by
 Commit changed from 052721a72568247cafc5abff310ad3478f999961 to 4dd721a971b8606497cc046cf19e2706666cdf37
Branch pushed to git repo; I updated commit sha1. New commits:
4dd721a  trac #16820 fixing doc inclusion once again (no .py)

comment:19 Changed 7 years ago by
 Commit changed from 4dd721a971b8606497cc046cf19e2706666cdf37 to ee3edc49855a1dd98bca0fad32f38897c5d9a3df
Branch pushed to git repo; I updated commit sha1. New commits:
ee3edc4  attempt at using from_vector in _element_constructor_; this causes weird errors I have yet to understand

comment:20 Changed 7 years ago by
See my last commit. Errors:
********************************************************************** File "src/sage/algebras/lie_algebras/lie_algebra_element.py", line 179, in sage.algebras.lie_algebras.lie_algebra_element.LieAlgebraElementWrapper.__ne__ Failed example: L.zero() == 0 Expected: True Got: False ********************************************************************** File "src/sage/algebras/lie_algebras/lie_algebra_element.py", line 181, in sage.algebras.lie_algebras.lie_algebra_element.LieAlgebraElementWrapper.__ne__ Failed example: L.zero() != 0 Expected: False Got: True **********************************************************************
Also, are you sure you meant to keep the dot in sage/algebras/lie_algebras/examples.
?
comment:21 Changed 7 years ago by
 Commit changed from ee3edc49855a1dd98bca0fad32f38897c5d9a3df to eb81ce8a62555f2157475b4190190feaaaa86e6a
Branch pushed to git repo; I updated commit sha1. New commits:
eb81ce8  document brokenness

comment:22 Changed 7 years ago by
 Commit changed from eb81ce8a62555f2157475b4190190feaaaa86e6a to 87a227f1248a94beabd834007d458a505e753e46
comment:23 followup: ↓ 25 Changed 7 years ago by
It at least covers all 3dim Lie algebras over char 0 fields (up to isomorphism); I will check when I get home what generality they meant (and put a reference if you think it's necessary).
Also for this change:
+ if hasattr(self, "module") and x in self.module():
+ return self.from_vector(x)
I think you should do
try: if x in self.module(): return self.from_vector(x) except AttributeError: pass
For the FromAssociative.lift
...well there's some ambiguity there as to whether we want to lift to the UEA or just some enveloping algebra. I opted to lift just to the defining algebra (which I forgot to change in the docs) as this seemed the most natural. We will also have to deal with subalgebras and a lift
there.
For the tested methods, it was originally returning an element from a matrix Lie algebra, but through refactoring this had changed. This can be remedied by changing the 3 to 1, i.e.: L = lie_algebras.three_dimensional_by_rank(QQ, 1)
.
I'm also not happy with the corner cases being allowed as all properties become vacuous.
Anyways, I'll make changes tomorrow as I will let Sage compile tonight when I get back home.
comment:24 Changed 7 years ago by
 Commit changed from 87a227f1248a94beabd834007d458a505e753e46 to 8c5361b202dd68b31be6eb414b437badceb3fa76
Branch pushed to git repo; I updated commit sha1. New commits:
8c5361b  meanwhile, fix _acted_upon_ bug

comment:25 in reply to: ↑ 23 ; followup: ↓ 27 Changed 7 years ago by
 Milestone changed from sage6.6 to sage6.7
Replying to tscrim:
It at least covers all 3dim Lie algebras over char 0 fields (up to isomorphism); I will check when I get home what generality they meant (and put a reference if you think it's necessary).
Ah, please add the reference. That's quite an interesting result.
Also for this change:
+ if hasattr(self, "module") and x in self.module(): + return self.from_vector(x)
I think you should do
try: if x in self.module(): return self.from_vector(x) except AttributeError: pass
Does this do something different or is it just more pythonic? I am worried about this tryexcept game as I can't tell whether the AttributeError? comes from self.module() or from the self.from_vector(x).
For the
FromAssociative.lift
...well there's some ambiguity there as to whether we want to lift to the UEA or just some enveloping algebra. I opted to lift just to the defining algebra (which I forgot to change in the docs) as this seemed the most natural. We will also have to deal with subalgebras and alift
there.
I am in favor of renaming it then. The abstract lift
lazy_attribute in sage/categories/lie_algebras.py
explicitly speaks of the UEA in its doc.
For the tested methods, it was originally returning an element from a matrix Lie algebra, but through refactoring this had changed. This can be remedied by changing the 3 to 1, i.e.:
L = lie_algebras.three_dimensional_by_rank(QQ, 1)
.I'm also not happy with the corner cases being allowed as all properties become vacuous.
The corner cases need to be allowed, and I've already caught at least 2 bugs by inserting tests for them.
New commits:
8c5361b  meanwhile, fix _acted_upon_ bug

comment:26 Changed 7 years ago by
 Commit changed from 8c5361b202dd68b31be6eb414b437badceb3fa76 to 021ed1e344f8a30bc08407ac9069cb950429ff6d
Branch pushed to git repo; I updated commit sha1. New commits:
021ed1e  more tests

comment:27 in reply to: ↑ 25 ; followup: ↓ 29 Changed 7 years ago by
Replying to darij:
Replying to tscrim:
It at least covers all 3dim Lie algebras over char 0 fields (up to isomorphism); I will check when I get home what generality they meant (and put a reference if you think it's necessary).
Ah, please add the reference. That's quite an interesting result.
IIRC, it's mostly just doing some linear algebra, going rank by rank, and using the constraint by the Jacobi identity. However I will add it.
Also for this change:
+ if hasattr(self, "module") and x in self.module(): + return self.from_vector(x)
I think you should do
try: if x in self.module(): return self.from_vector(x) except AttributeError: passDoes this do something different or is it just more pythonic? I am worried about this tryexcept game as I can't tell whether the AttributeError? comes from self.module() or from the self.from_vector(x).
It is more pythonic (and faster, but not noticably). You are the one who wanted the rule of 3s contract, and because of the category almost every Lie algebra which has a module attribute also has a from_vector
. Also I'd say the failure is more graceful this way as if either one is not implemented, then it results in an unable to do the coercion/conversion type error.
For the
FromAssociative.lift
...well there's some ambiguity there as to whether we want to lift to the UEA or just some enveloping algebra. I opted to lift just to the defining algebra (which I forgot to change in the docs) as this seemed the most natural. We will also have to deal with subalgebras and alift
there.I am in favor of renaming it then. The abstract
lift
lazy_attribute insage/categories/lie_algebras.py
explicitly speaks of the UEA in its doc. Alift
that can go anywhere depending on the concrete algebra will be rather useless.
Renaming which one? Also it's far from useless to lift to some enveloping algebra as you would want to convert between say, SGA and the Lie algebra.
For the tested methods, it was originally returning an element from a matrix Lie algebra, but through refactoring this had changed. This can be remedied by changing the 3 to 1, i.e.:
L = lie_algebras.three_dimensional_by_rank(QQ, 1)
.Nope, they are still not wrappers.
Whoops, yes. Do sl(QQ, 2, representation='matrix')
or one of the upper triangular matrix Lie algebras.
I'm also not happy with the corner cases being allowed as all properties become vacuous.
The corner cases need to be allowed, and I've already caught at least 2 bugs by inserting tests for them.
No, they don't have to be allowed. In fact, because they are the empty set, they aren't really welldefined to me.
comment:28 Changed 7 years ago by
 Commit changed from 021ed1e344f8a30bc08407ac9069cb950429ff6d to c1838221b369f47e407ffcd95fd53bd4f239e21d
Branch pushed to git repo; I updated commit sha1. New commits:
c183822  all doctests pass again :)

comment:29 in reply to: ↑ 27 Changed 7 years ago by
Replying to tscrim:
IIRC, it's mostly just doing some linear algebra, going rank by rank, and using the constraint by the Jacobi identity. However I will add it.
A lot of Lie theory is just doing some linear algebra... Linear algebra does a lot.
It is more pythonic (and faster, but not noticably). You are the one who wanted the rule of 3s contract, and because of the category almost every Lie algebra which has a module attribute also has a
from_vector
. Also I'd say the failure is more graceful this way as if either one is not implemented, then it results in an unable to do the coercion/conversion type error.
You're right  fixed!
For the
FromAssociative.lift
...well there's some ambiguity there as to whether we want to lift to the UEA or just some enveloping algebra. I opted to lift just to the defining algebra (which I forgot to change in the docs) as this seemed the most natural. We will also have to deal with subalgebras and alift
there.
So you have 3 different meanings of lift now:
 lift from g to U(g);
 lift from g to *some* associative algebra containing g;
 lift from g to a bigger Lie algebra.
I have no idea how I am to rely on a method which can do either of these three things depending on the implementation (and possibly on the MRO?). These warrant distinct names.
Renaming which one?
Either.
Whoops, yes. Do
sl(QQ, 2, representation='matrix')
or one of the upper triangular matrix Lie algebras.
Thanks. And you know what's funny? The try/except change actually fixed the == 0 tests...
but caused some new crap:
sage: L = lie_algebras.sl(QQ, 2, representation='matrix') sage: L(2) [2 0] [0 2]
This is not in sl(2)!
Although, this is tolerable due to how this version of sl(2) is defined (as a generate of E, F and H).
No, they don't have to be allowed. In fact, because they are the empty set, they aren't really welldefined to me.
0dimensional Lie algebras exist and sometimes you get them by quotienting or restricting. It isn't mathematically reasonable to exclude them.
New commits:
c183822  all doctests pass again :)

comment:30 Changed 7 years ago by
IIRC, how I got around the subalgebra lifting was calling it lift_ambient
. Perhaps what we should do is have it take an optional argument which calls the respective coercion and has some default. Maybe ask Nicolas?
You're right, but it's the one consisting of {0}
. However this is not the empty set like the set of 0x0
matrices the corner cases construct.
I'm getting on my flight now. Talk to you when I'm back in California.
comment:31 Changed 7 years ago by
 Keywords sd67 added
lift_ambient
sounds good  now I'd just wish the other two lifts would have different names.
What do you mean by "not the empty set like the set of 0x0 matrices"? There is a unique 0x0 matrix, and it is strictly uppertriangular.
Yes, talk to you later, and thanks for all the quick responses so far  they're very helpful.
Meanwhile, here is an enigma for other people to solve:
sage: L = lie_algebras.three_dimensional_by_rank(QQ, 3) sage: L in Modules(QQ) True sage: L.zero() == 0 True sage: L.zero() == QQ(0) False
comment:32 Changed 7 years ago by
 Commit changed from c1838221b369f47e407ffcd95fd53bd4f239e21d to c75e37bbdc5a9208f04bd4b09a1f049aacf0bb32
Branch pushed to git repo; I updated commit sha1. New commits:
dce923e  Fixing cases when the action should also precompose with a coercion.

6b2e9bb  Added test which checks the action map.

9b14aa5  Change in docstring.

75232f0  Merge branch 'public/coercion/fix_actions18221' of git://trac.sagemath.org/sage into lie2

c75e37b  with trac 18221 merged in, some stopgaps removed

comment:33 Changed 7 years ago by
 Commit changed from c75e37bbdc5a9208f04bd4b09a1f049aacf0bb32 to 3032fc09f15c90b61a3cf9b146e90b000c15fa4c
Branch pushed to git repo; I updated commit sha1. New commits:
3032fc0  product of Lie algebra elements with base ring elements now doesn't lift into the universal envelope

comment:34 Changed 7 years ago by
Here is a problem with using lift
for any kind of lift:
sage: L = lie_algebras.sl(QQ, 2, representation='matrix') sage: E, F, H = L.gens() sage: E [0 1] [0 0] sage: F [0 0] [1 0] sage: H [ 1 0] [ 0 1] sage: E * H [ 0 1] [ 0 0]
But E * H should not be E if it's really to mean the element E * H of the universal envelope.
Also, it's black on white (green on black?) here:
def universal_enveloping_algebra(self): """ Return the universal enveloping algebra of ``self``. EXAMPLES:: sage: L = LieAlgebras(QQ).FiniteDimensional().WithBasis().example() sage: L.universal_enveloping_algebra() Noncommutative Multivariate Polynomial Ring in b0, b1, b2 over Rational Field, ncrelations: {} :: sage: L = LieAlgebra(QQ, 3, 'x', abelian=True) sage: L.universal_enveloping_algebra() Multivariate Polynomial Ring in x0, x1, x2 over Rational Field .. SEEALSO:: :meth:`lift` """ return self.lift.codomain()
The codomain of the lift has to be the UEA.
comment:35 Changed 7 years ago by
 Commit changed from 3032fc09f15c90b61a3cf9b146e90b000c15fa4c to bf2eb372d2135972a1e646d1120772da33dc8013
Branch pushed to git repo; I updated commit sha1. New commits:
bf2eb37  same optimization as in trac 17098, plus getting rid of todos

comment:36 Changed 7 years ago by
 Commit changed from bf2eb372d2135972a1e646d1120772da33dc8013 to 09281457c8cf34a075be3b72ae88296a75c54d93
Branch pushed to git repo; I updated commit sha1. New commits:
0928145  trivial edit

comment:37 followup: ↓ 38 Changed 7 years ago by
 Dependencies #16819 deleted
Sorry for the delay in getting back to you; traveling plus clearing off my built up work.
comment:29 I think the issue is that there isn't enough checking of coefficients and maybe a default assumption somewhere that algebras are unital? Perhaps it's also lifting up in some fashion? I'll take a look at that this week.
comment:31
I would say the set of 0x0 matrices is the empty set, not the set containing a unique element, the empty matrix. As for the oddity, I think we need a better/more uniform system for equality for things that behave like 0. At the very least, this is an issue with CombinatorialFreeModuleElement
that deserves a separate ticket:
sage: C = CombinatorialFreeModule(ZZ, ['a','b']) sage: C.zero() == 0 True sage: C.zero() == QQ(0) False
comment:34
Yes, that is definitely bad. I'm thinking we should have lift
always return an element of the UEA and state that explicitly. For the lifting to the defining associative algebra, perhaps call that lift_associative
?
comment:38 in reply to: ↑ 37 ; followup: ↓ 39 Changed 7 years ago by
Sorry for the delay in getting back to you; traveling plus clearing off my built up work.
Don't worry. I'm again swamped in work, so the sorriness is mutual.
Replying to tscrim:
comment:29 I think the issue is that there isn't enough checking of coefficients and maybe a default assumption somewhere that algebras are unital? Perhaps it's also lifting up in some fashion? I'll take a look at that this week.
Forget about this part  I've since noticed that you define sl(2) as a Lie subalgebra of an associative algebra generated by a subset, and since you don't have methods which compute such a thing exactly, there is no surprise that it accepts input too liberally.
comment:31 I would say the set of 0x0 matrices is the empty set, not the set containing a unique element, the empty matrix.
There are very good reasons for considering it a oneelement set. Matrices of size m \times n correspond to morphisms R^n \to R^m
. How many morphisms are there from R^0
to R^0
(that is, from 0 to 0) ? One  the zero morphism.
As for the oddity, I think we need a better/more uniform system for equality for things that behave like 0. At the very least, this is an issue with
CombinatorialFreeModuleElement
that deserves a separate ticket:sage: C = CombinatorialFreeModule(ZZ, ['a','b']) sage: C.zero() == 0 True sage: C.zero() == QQ(0) False
+1.
comment:34 Yes, that is definitely bad. I'm thinking we should have
lift
always return an element of the UEA and state that explicitly. For the lifting to the defining associative algebra, perhaps call thatlift_associative
?
That's a good idea.
The Lie subalgebra of an associative algebra generated by a subset will, so far, have no UEA, since we don't compute its full ground set. But now you made me wonder if we really need the Lie subalgebra of an associative algebra generated by a subset... If we don't compute its ground set, and only let the user compute "inside" it, then why don't we just use the whole associative algebra as a Lie algebra? The generators seem to be useless...
comment:39 in reply to: ↑ 38 Changed 7 years ago by
Replying to darij:
Replying to tscrim:
comment:31 I would say the set of 0x0 matrices is the empty set, not the set containing a unique element, the empty matrix.
There are very good reasons for considering it a oneelement set. Matrices of size m \times n correspond to morphisms
R^n \to R^m
. How many morphisms are there fromR^0
toR^0
(that is, from 0 to 0) ? One  the zero morphism.
I would say that this is a degenerate case of the general equality, but fair enough. More fun with corner cases that no one will likely construct except for corner case testing... Will fix.
As for the oddity, I think we need a better/more uniform system for equality for things that behave like 0. At the very least, this is an issue with
CombinatorialFreeModuleElement
that deserves a separate ticket:sage: C = CombinatorialFreeModule(ZZ, ['a','b']) sage: C.zero() == 0 True sage: C.zero() == QQ(0) False+1.
This is #18251.
comment:34 Yes, that is definitely bad. I'm thinking we should have
lift
always return an element of the UEA and state that explicitly. For the lifting to the defining associative algebra, perhaps call thatlift_associative
?That's a good idea.
The Lie subalgebra of an associative algebra generated by a subset will, so far, have no UEA, since we don't compute its full ground set. But now you made me wonder if we really need the Lie subalgebra of an associative algebra generated by a subset... If we don't compute its ground set, and only let the user compute "inside" it, then why don't we just use the whole associative algebra as a Lie algebra? The generators seem to be useless...
Then you'd have gl_{n} = sl_{n} and there will be Lie subalgebras eventually implemented. More generally, the UEA of the (Lie) subalgebra will be a (assoc) subalgebra of the UEA of the ambient Lie algebra. However, we will need general support for subalgebras, which I don't think #11111 provides...
comment:40 Changed 7 years ago by
 Commit changed from 09281457c8cf34a075be3b72ae88296a75c54d93 to 301ddb58fcbe191708c915f11985c37b73667922
comment:41 Changed 7 years ago by
 Commit changed from 301ddb58fcbe191708c915f11985c37b73667922 to 690e2f50e7d4d0b7446738a3a60bd0faa8bba540
Branch pushed to git repo; I updated commit sha1. New commits:
690e2f5  Merge branch 'public/lie_algebras/fd_structure_coeff16820' into 6.8.b0

comment:42 Changed 6 years ago by
 Status changed from needs_review to needs_work
one failing doctest, see patchbot report
comment:43 Changed 6 years ago by
 Commit changed from 690e2f50e7d4d0b7446738a3a60bd0faa8bba540 to 7a768e82f1eb1ccacf04169f8aadcbb4505eaae8
Branch pushed to git repo; I updated commit sha1. New commits:
a7f9800  Merge branch 'public/lie_algebras/fd_structure_coeff16820' of git://trac.sagemath.org/sage into public/lie_algebras/fd_structure_coeff16820

53aec59  Fixed trivial doctest failure.

7a768e8  Reworking the UEA for LieAlgebraFromAssociative.

comment:44 Changed 6 years ago by
 Status changed from needs_work to needs_review
I've fixed the issues with lift
and the universal enveloping algebra for LieAlgebraFromAssociative
.
Right now I'm going to leave the option to specify the generators when constructing a Lie algebra from an associative algebra. Once #17416 (subalgebras) is done, it will directly create the corresponding Lie subalgebra.
comment:45 Changed 6 years ago by
 Commit changed from 7a768e82f1eb1ccacf04169f8aadcbb4505eaae8 to 2f6b1e40e53f0039e5786a9390bdad1c9d2a7217
comment:46 Changed 6 years ago by
Whoops :p
, thanks Frederic.
comment:47 Changed 6 years ago by
 Status changed from needs_review to needs_work
two failing doctests
comment:48 Changed 6 years ago by
 Milestone changed from sage6.7 to sage6.10
comment:49 Changed 6 years ago by
 Commit changed from 2f6b1e40e53f0039e5786a9390bdad1c9d2a7217 to 7c3978711e1ab0950e720b0bd7ba6e18c185114c
Branch pushed to git repo; I updated commit sha1. New commits:
7c39787  Merge branch 'public/lie_algebras/fd_structure_coeff16820' of trac.sagemath.org:sage into public/lie_algebras/fd_structure_coeff16820

comment:50 Changed 6 years ago by
 Commit changed from 7c3978711e1ab0950e720b0bd7ba6e18c185114c to 15e231797c8028a56d66eb19b067df90f2822822
Branch pushed to git repo; I updated commit sha1. New commits:
15e2317  Fixing doctests due to output order.

comment:51 Changed 6 years ago by
 Commit changed from 15e231797c8028a56d66eb19b067df90f2822822 to b38f9838d5ebfcd120587543d60333c46ad3865a
Branch pushed to git repo; I updated commit sha1. New commits:
b38f983  documentation improvements

comment:52 Changed 6 years ago by
 Commit changed from b38f9838d5ebfcd120587543d60333c46ad3865a to 090d308cd47fb5dfec0acc935972af18e2164ded
Branch pushed to git repo; I updated commit sha1. New commits:
090d308  oops

comment:53 Changed 6 years ago by
 Dependencies set to #17035
comment:54 Changed 6 years ago by
 Commit changed from 090d308cd47fb5dfec0acc935972af18e2164ded to eedbb63c2964f52cb513666e67d73b56adcb1752
comment:55 Changed 6 years ago by
 Commit changed from eedbb63c2964f52cb513666e67d73b56adcb1752 to 60031718f3b44571e9f83593aa7673a5cbb997d3
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
22f8b77  18411: proofreading

4a9fd81  Trac 18411: merge Sage 6.10.beta0

525d038  Trac 18411: Fix doctest in colored_permutations.py

41af762  Trac 18411: pass keywords in __call__

6e27dde  Trac 18411: merge sage6.10.beta1

b21396c  Merge branch '18411' into HEAD

b32e479  further changes (mostly doc)

dbb5789  Further fixes

e2fb14a  remove _test_keytype since submodules of free modules are currently failing it

6003171  Merge branch 'public/categories/cleanup_CFM_modules_wBasis18066' of trac.sagemath.org:sage into public/lie_algebras/fd_structure_coeff16820

comment:56 Changed 6 years ago by
 Dependencies changed from #17035 to #17035 #18411 #18066
 Reviewers set to Darij Grinberg
 Status changed from needs_work to needs_review
comment:57 Changed 6 years ago by
 Commit changed from 60031718f3b44571e9f83593aa7673a5cbb997d3 to 10ce8ff4c84248795e99f807aac2772c33e03cd6
Branch pushed to git repo; I updated commit sha1. New commits:
10ce8ff  Fixing some things from recent positive review/merges.

comment:58 Changed 6 years ago by
 Commit changed from 10ce8ff4c84248795e99f807aac2772c33e03cd6 to cdb2e5b9476356aa9f91d372a51aa802921f0e62
Branch pushed to git repo; I updated commit sha1. New commits:
cdb2e5b  review heisenberg.py

comment:59 Changed 6 years ago by
 Commit changed from cdb2e5b9476356aa9f91d372a51aa802921f0e62 to eda8d898c413e6244f515d72f6cbdca4aa2a763e
Branch pushed to git repo; I updated commit sha1. New commits:
eda8d89  review virasoro.py

comment:60 Changed 6 years ago by
 Commit changed from eda8d898c413e6244f515d72f6cbdca4aa2a763e to be4a02edb5f542e8bb988243cdd80d4ff33b384f
comment:61 Changed 6 years ago by
 Commit changed from be4a02edb5f542e8bb988243cdd80d4ff33b384f to b8be3f4298913132df18d3230186f93686561223
Branch pushed to git repo; I updated commit sha1. New commits:
b8be3f4  review heisenberg.py again; coercion still relies on unstaged changes to module morphisms

comment:62 Changed 6 years ago by
 Dependencies changed from #17035 #18411 #18066 to #17035 #18411 #18066 #10672
comment:63 Changed 6 years ago by
 Commit changed from b8be3f4298913132df18d3230186f93686561223 to 6815f1c02c6a13cef96d542328d3248e06ca9fb9
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
6815f1c  review heisenberg.py again; coercion still relies on unstaged changes to module morphisms

comment:64 Changed 6 years ago by
 Commit changed from 6815f1c02c6a13cef96d542328d3248e06ca9fb9 to 573e0748daeb844cb79792c08a5c851ea047e7f1
Branch pushed to git repo; I updated commit sha1. New commits:
b8be3f4  review heisenberg.py again; coercion still relies on unstaged changes to module morphisms

96004df  Darij...

d94f997  Some last compatibility issues with modules with basis (hopefully).

c986ca3  Merge branch 'public/categories/last_compatibility10672' into public/lie_algebras/fd_structure_coeff16820

573e074  Merge branch 'public/lie_algebras/fd_structure_coeff16820' of trac.sagemath.org:sage into public/lie_algebras/fd_structure_coeff16820

comment:65 Changed 6 years ago by
 Status changed from needs_review to needs_work
 Summary changed from Implement ABCs for Lie algebras and finite dimensional given by structure cofficients to Implement ABCs for Lie algebras and finite dimensional given by structure coefficients
doc does not build
comment:66 Changed 6 years ago by
 Commit changed from 573e0748daeb844cb79792c08a5c851ea047e7f1 to 3f9a4a19a30d6749ab046160bc43988958c9490f
Branch pushed to git repo; I updated commit sha1. New commits:
3f9a4a1  Fixing the failing doctests and making the doc build.

comment:67 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:68 Changed 6 years ago by
 Commit changed from 3f9a4a19a30d6749ab046160bc43988958c9490f to b9930e6c4f3f099baf341b58dc81d50a00df4986
Branch pushed to git repo; I updated commit sha1. New commits:
b9930e6  some changes to standardization of names

comment:69 Changed 6 years ago by
I feel I don't understand the correct meaning of the arguments in a LieAlgebra
call very well. What is going wrong here: user error?
sage: L = LieAlgebra(QQ, 'x,y,z', {('x','y'):{'z':1}, ('y','z'):{'x':1}, ('z','x'):{'y':1}}, index_set=range(3)) sage: (x,y,z) = L.gens() sage: L.is_abelian() False sage: L[x,y] 0 sage: L[y,z] 0 sage: L[z,x] 0 sage: L[x,y] == L.zero() == L[y,z] == L[x,y] True
comment:70 Changed 6 years ago by
It is user error as the keys for the structure coefficients should be the index set. However, I could (should) change things so that it doesn't matter which you use.
comment:71 Changed 6 years ago by
Ah! You're right, this is user error. I got confused by the fact that the names come before the index_set.
comment:72 Changed 6 years ago by
However I'm fixing the input so that it mends the input to use the indexing set.
comment:73 Changed 6 years ago by
...in structure_coefficients.py
.
comment:74 Changed 6 years ago by
 Commit changed from b9930e6c4f3f099baf341b58dc81d50a00df4986 to 60f743a2d8f8d684e4e636d6bdf329fae9967cb2
Branch pushed to git repo; I updated commit sha1. New commits:
60f743a  Parsing the input so that it uses the indexing set.

comment:75 Changed 6 years ago by
With this last commit, we have:
sage: sage: L = LieAlgebra(QQ, 'x,y,z', {('x','y'):{'z':1}, ('y','z'):{'x':1}, ('z','x'):{'y':1}}, index_set=range(3)) sage: sage: (x,y,z) = L.gens() sage: sage: L[x,y] L[2]
comment:76 Changed 6 years ago by
 Milestone changed from sage6.10 to sage7.0
two failing doctests, see patchbot report
comment:77 Changed 6 years ago by
 Commit changed from 60f743a2d8f8d684e4e636d6bdf329fae9967cb2 to 50f4441019eeb080c7e700e2853f9b6aa564cf7c
comment:78 Changed 6 years ago by
It seems that one doctest in Virasoro is fragile, namely the order of the terms is not always the same. I have tried to correct this doctest according to a previous patchbot report, and now it is failing again.
comment:79 Changed 6 years ago by
 Dependencies #17035 #18411 #18066 #10672 deleted
 Milestone changed from sage7.0 to sage7.1
 Status changed from needs_review to needs_work
Minor comment: when adding new files, please use the standard copyright template from http://doc.sagemath.org/html/en/developer/coding_basics.html#headingsofsagelibrarycodefiles.
comment:80 Changed 6 years ago by
 Reviewers Darij Grinberg deleted
Removing my name from the reviewer field, since I'm afraid it creates a false impression that this ticket is being taken care of. I currently don't believe that I will find the time necessary for this until Summer :/
comment:81 Changed 6 years ago by
 Reviewers set to Darij Grinberg
However you have done a fair amount of review of this ticket and deserve to be put on as a reviewer. I think your message saying you won't likely be able to get to this until summer is sufficient for anyone following this.
comment:82 Changed 6 years ago by
 Commit changed from 50f4441019eeb080c7e700e2853f9b6aa564cf7c to f1514bd21164a21c8ff90adf91bb036605fc1d85
Branch pushed to git repo; I updated commit sha1. New commits:
f1514bd  Merge branch 'public/lie_algebras/fd_structure_coeff16820' into 7.3.b2

comment:83 Changed 6 years ago by
 Commit changed from f1514bd21164a21c8ff90adf91bb036605fc1d85 to a016182b1fcb8dcb4b29840be98436dda8531464
comment:84 Changed 6 years ago by
 Keywords days74 added
 Milestone changed from sage7.1 to sage7.3
comment:85 Changed 6 years ago by
apparently, doc does not build.
comment:86 Changed 6 years ago by
 Commit changed from a016182b1fcb8dcb4b29840be98436dda8531464 to 6fcb3726142ab68451acc682d523b264bc1e319f
Branch pushed to git repo; I updated commit sha1. New commits:
6fcb372  Travis' fix for documentation

comment:87 Changed 6 years ago by
 Status changed from needs_work to needs_review
I did some basic looking over things and everything seems to work. Still needs to have the mathematics looked at and then the code further tested.
comment:88 Changed 6 years ago by
 Commit changed from 6fcb3726142ab68451acc682d523b264bc1e319f to 00816c6f058fcc1603cd4d8fdb9db495841f8ea5
Branch pushed to git repo; I updated commit sha1. New commits:
00816c6  Merge branch 'public/lie_algebras/fd_structure_coeff16820' into 7.3.b7

comment:89 Changed 6 years ago by
one doctest complains about deprecation of generator_cmp
comment:90 Changed 6 years ago by
 Commit changed from 00816c6f058fcc1603cd4d8fdb9db495841f8ea5 to d5e618b4517c8766181a276da80f4e34080f60b6
Branch pushed to git repo; I updated commit sha1. New commits:
d5e618b  abc of Lie algebras, use key rather than cmp to sort ; and sorting_key

comment:91 Changed 5 years ago by
 Commit changed from d5e618b4517c8766181a276da80f4e34080f60b6 to e3133954dd66f164586e5528f5825bbedb6f9e3a
Branch pushed to git repo; I updated commit sha1. New commits:
e313395  Merge branch 'public/lie_algebras/fd_structure_coeff16820' in 7.3.b9

comment:92 Changed 5 years ago by
Travis and Darij, do you think that this still needs a lot of work ? That would be great to have in sage !
comment:93 Changed 5 years ago by
I am in the process of doing a rewrite of the underlying implementation from using CFM to wrapping the usual free modules since all of the algorithms rely on linear algebra. However, this doesn't need much work beyond that; just more of a check of the doc and basic math. So I can postpone by rewrite to a followup ticket if you want to finish the review now.
comment:94 Changed 5 years ago by
I don't know how much work this needs :/ truth be told, I've only reviewed some of the more mathematical parts of it. I was planning to do the rest of it when I'm in Minneapolis (it is so much easier when the author is nearby), but if you (Frederic, or whoever else) beat me to it, I'd certainly be thankful :)
comment:95 Changed 5 years ago by
 Commit changed from e3133954dd66f164586e5528f5825bbedb6f9e3a to e6851853a6de6e65cefabcd27b7ec20bf5ecfb25
Branch pushed to git repo; I updated commit sha1. New commits:
224da75  Merge branch 'develop' into public/lie_algebras/fd_structure_coeff16820

8c0f1ab  Merge branch 'develop' into public/lie_algebras/fd_structure_coeff16820

16e36a9  Refactor to use free modules for findim Lie algebras w/ structure coeffs; a bit more work needed still.

e685185  Merge branch 'public/lie_algebras/fd_structure_coeff16820' of trac.sagemath.org:sage into public/lie_algebras/fd_structure_coeff16820

comment:96 Changed 5 years ago by
Here's my work so far. There's still a few things that need to be fixed, but by far, the refactoring seems to be working. I hope to finish the rest over the next week or so.
comment:97 Changed 5 years ago by
 Commit changed from e6851853a6de6e65cefabcd27b7ec20bf5ecfb25 to 9f33f5d2da42d3c8619a1690d655579884418f2c
comment:98 Changed 5 years ago by
 Commit changed from 9f33f5d2da42d3c8619a1690d655579884418f2c to 394c3f0eee14dc17129930c459d914c6252e5961
Branch pushed to git repo; I updated commit sha1. New commits:
394c3f0  Not creating a new vector at every step in _bracket_.

comment:99 Changed 5 years ago by
 Milestone changed from sage7.3 to sage7.4
 Status changed from needs_review to needs_work
Okay, that should fix all of the outstanding problems with my rebase. The only thing left is to add doctests, fix the docbuild, change the failing doctests (which are caused by those tests being for a different class), and remove any cruft that might have built up.
I think I will also add a class for infinitedimensional Lie algebras that uses LieAlgebraElement
(which should be renamed) that behaves somewhat like a CombinatorialFreeModule
, but inherits from LieAlgebra
. This will serve as another common ABC for the Heisenberg and Virasoro Lie algebras, loop algebras, etc.
I also Cythonized the element class (for Lie algebras w/ structure coefficients) and tried to make it so those are as fast as can be.
comment:100 Changed 5 years ago by
 Commit changed from 394c3f0eee14dc17129930c459d914c6252e5961 to 2e6692c257dbcf9b48252adc78adb41feb86de41
comment:101 Changed 5 years ago by
 Commit changed from 2e6692c257dbcf9b48252adc78adb41feb86de41 to af54ea281be2afdd7ddcc47527900ae152c111d3
Branch pushed to git repo; I updated commit sha1. New commits:
af54ea2  Cleaning up some (currently) unused imports.

comment:102 Changed 5 years ago by
 Keywords days79 added
 Status changed from needs_work to needs_review
Okay, back and fully ready for review.
comment:103 Changed 5 years ago by
 Commit changed from af54ea281be2afdd7ddcc47527900ae152c111d3 to d333710a1c9322000b5ae338ca037d3beb647a3e
Branch pushed to git repo; I updated commit sha1. New commits:
d333710  Omission of abelian Lie algebras from doc index, and citation fix.

comment:104 Changed 5 years ago by
 Commit changed from d333710a1c9322000b5ae338ca037d3beb647a3e to eff47f6b34d6b9e987912cf8236a895b77596413
Branch pushed to git repo; I updated commit sha1. New commits:
eff47f6  standardize_names_index_set: Fix doctest and better documentation

comment:105 Changed 5 years ago by
In comment:103 and comment:104 I reviewed the src/sage/structure/indexed_generators.py
and src/sage/algebras/lie_algebras/abelian.py
files, and corrected the minor errors I have found there.
Reviewing the entire ticket might be too much for me, but it would be great to have Lie algebras in Sage.
comment:106 Changed 5 years ago by
there is an xrange, and many .iteritems (not good for py3)
comment:107 Changed 5 years ago by
 Commit changed from eff47f6b34d6b9e987912cf8236a895b77596413 to 965c70b1ef515c23bf6ca3be3ff6bf8a9d03c94b
comment:108 Changed 5 years ago by
 Commit changed from 965c70b1ef515c23bf6ca3be3ff6bf8a9d03c94b to 7c92fd10fd6a4557c98e908d5d8051d57c9ac630
comment:109 Changed 5 years ago by
 Commit changed from 7c92fd10fd6a4557c98e908d5d8051d57c9ac630 to 7bda1a49c73c19c3b18eb9fb0b07ece5f6323f65
comment:110 followup: ↓ 113 Changed 5 years ago by
 Reviewers changed from Darij Grinberg to Darij Grinberg, Eric Gourgoulhon
This ticket looks in good shape to me, with clear maths (as far as I can tell) and very high quality code. I've just a few comments:
 The documentation of classes
MatrixLieAlgebraFromAssociative
andLieAlgebraMatrixWrapper
is reduced to "Initialize self.", which certainly could be improved, even if the class names are quite explicit.
 There is a typo in the reference [deGraaf2000]: "Mathemtaical" > "Mathematical"
 Some infinitedimensional Lie algebras lack of a
dimension
method:sage: R = FreeAlgebra(QQ, 3) sage: dim(R) +Infinity sage: L = LieAlgebra(associative=R) sage: dim(L) AttributeError: 'LieAlgebraFromAssociative_with_category' object has no attribute 'dimension'
Btw, shouldn'tdimension
be added toParentMethods
in the Lie algebra category?  Regarding Heisenberg Lie algebras:
sage: L = lie_algebras.Heisenberg(QQ, 2) sage: L.lie_algebra_generators() Finite family {'p2': p2, 'q1': q1, 'p1': p1, 'q2': q2, 'z': z} sage: L.gens() (p1, p2, q1, q2)
Shouldn't z be part of the tuple of generators returned bygens
?
 The patchbot is complaining about "Python 3 incompatible code" in
lie_algebra_element.pyx
, namely the use ofiteritems
. Has this not been fixed byfrom six import iteritems
because it is a Cython file?
 Just for curiosity: why are you constructing all Lie algebras by invoking the base class
LieAlgebra
, relying onLieAlgebra.__classcall_private__
to dispatch to the actual subclass corresponding to the input? Couldn't you use a functionLieAlgebra
to do this, as we did for manifolds (cf. the functionManifold
)? (possibly renaming the classLieAlgebra
to e.g.LieAlgebraBase
to avoid any confusion). A drawback of the current setting is that the inline documentation obtained fromLieAlgebra?
returns the__init__
docstring:INPUT: * "R"  the base ring * "names"  (optional) the names of the generators * "category"  the category of the Lie algebra; the default is the category of Lie algebras over "R"
which does not correspond to the actual arguments to be passed toLieAlgebra
(because of the__classcall_private__
mechanism) and therefore may confuse the user. If you want to keepLieAlgebra.__classcall_private__
, a solution could be to set theINPUT
section in the docstring of theLieAlgebra
class.
comment:111 Changed 5 years ago by
Another minor remark: the ticket title looks strange to me: shouldn't a name ("Lie algebras", I guess) be inserted behind "finite dimensional"?
comment:112 Changed 5 years ago by
 Commit changed from 7bda1a49c73c19c3b18eb9fb0b07ece5f6323f65 to cf88fee19593c1dc07b7e11a64a1b7cfa1c0f148
comment:113 in reply to: ↑ 110 ; followup: ↓ 114 Changed 5 years ago by
 Description modified (diff)
 Milestone changed from sage7.4 to sage8.0
 Summary changed from Implement ABCs for Lie algebras and finite dimensional given by structure coefficients to Implement ABCs and categories for Lie algebras and finite dimensional Lie algebras given by structure coefficients
Replying to egourgoulhon:
This ticket looks in good shape to me, with clear maths (as far as I can tell) and very high quality code.
Thank you for doing the review.
 The documentation of classes
MatrixLieAlgebraFromAssociative
andLieAlgebraMatrixWrapper
is reduced to "Initialize self.", which certainly could be improved, even if the class names are quite explicit.
Fixed by adding a docstring stub.
 There is a typo in the reference [deGraaf2000]: "Mathemtaical" > "Mathematical"
Fixed.
 Some infinitedimensional Lie algebras lack of a
dimension
method:sage: R = FreeAlgebra(QQ, 3) sage: dim(R) +Infinity sage: L = LieAlgebra(associative=R) sage: dim(L) AttributeError: 'LieAlgebraFromAssociative_with_category' object has no attribute 'dimension'Btw, shouldn'tdimension
be added toParentMethods
in the Lie algebra category?
Fixed. Really it should belong to the ModulesWithBasis
category (see #22629), but I moved it up to LieAlgebrasWithBasis
category for now.
 Regarding Heisenberg Lie algebras:
sage: L = lie_algebras.Heisenberg(QQ, 2) sage: L.lie_algebra_generators() Finite family {'p2': p2, 'q1': q1, 'p1': p1, 'q2': q2, 'z': z} sage: L.gens() (p1, p2, q1, q2)Shouldn't z be part of the tuple of generators returned bygens
?
I went the other way and dropped z
from lie_algebra_generators
since I wanted a more minimal generating set.
 The patchbot is complaining about "Python 3 incompatible code" in
lie_algebra_element.pyx
, namely the use ofiteritems
. Has this not been fixed byfrom six import iteritems
because it is a Cython file?
Yes, that is correct. I did some other small improvements from things I've learned about Cython recently.
 Just for curiosity: why are you constructing all Lie algebras by invoking the base class
LieAlgebra
, relying onLieAlgebra.__classcall_private__
to dispatch to the actual subclass corresponding to the input? Couldn't you use a functionLieAlgebra
to do this, as we did for manifolds (cf. the functionManifold
)? (possibly renaming the classLieAlgebra
to e.g.LieAlgebraBase
to avoid any confusion). A drawback of the current setting is that the inline documentation obtained fromLieAlgebra?
returns the__init__
docstring:[snip]
which does not correspond to the actual arguments to be passed to
LieAlgebra
(because of the__classcall_private__
mechanism) and therefore may confuse the user. If you want to keepLieAlgebra.__classcall_private__
, a solution could be to set theINPUT
section in the docstring of theLieAlgebra
class.
Because this is both the main entry point and the common base class, so there is no reason to separate the two IMO. It also avoids documentation duplication. Additionally the main docstring is (or at least should be) the classlevel docstring, which has lots of different possible inputs first as examples. I agree it is a minor drawback that the __init__
does get included, but it is way at the bottom. However, the possible inputs are quite varied, so writing an INPUT
block complicated.
comment:114 in reply to: ↑ 113 Changed 5 years ago by
Replying to tscrim:
Replying to egourgoulhon:
 Regarding Heisenberg Lie algebras: Shouldn't z be part of the tuple of generators returned by
gens
?I went the other way and dropped
z
fromlie_algebra_generators
since I wanted a more minimal generating set.
I agree. Accordingly, it may be worth to change the documentation of HeisenbergAlgebra.z()
to something like "the basis element z".
Still with Heisenberg algebras, we have
sage: L = lie_algebras.Heisenberg(QQ, 2) sage: L.center() AttributeError: 'HeisenbergAlgebra_with_category' object has no attribute 'basis_matrix'
Another issue arises because of the m[1]
in line 136 of heisenberg.py
::
sage: H = lie_algebras.Heisenberg(QQ, 10) sage: p10 = H.gen(9); p10 p10 sage: latex(p10) p_{1}
A drawback of the current setting is that the inline documentation obtained from
LieAlgebra?
returns the__init__
docstring, which does not correspond to the actual arguments to be passed toLieAlgebra
(because of the__classcall_private__
mechanism) and therefore may confuse the user. If you want to keepLieAlgebra.__classcall_private__
, a solution could be to set theINPUT
section in the docstring of theLieAlgebra
class.Because this is both the main entry point and the common base class, so there is no reason to separate the two IMO. It also avoids documentation duplication. Additionally the main docstring is (or at least should be) the classlevel docstring, which has lots of different possible inputs first as examples. I agree it is a minor drawback that the
__init__
does get included, but it is way at the bottom.
Thanks for these explanations.
However, the possible inputs are quite varied, so writing an
INPUT
block complicated.
But isn't it the point of a reference manual to provide an exhaustive list of all possible inputs? Maybe it is too much in this case; if you think that the provided examples perform a sufficient coverage, then it is fine for me.
comment:115 Changed 5 years ago by
Returning for a little bit of bikeshedding:
 "cosnstructed" in heisenberg.py.
 There is a bunch of TODOs in lie_algebra_element.pyx. Are they all still uptodate?
 In lie_algebra_element.pyx, is
self == 0 or y == 0
really the right thing? I thought we hadis_zero()
for testing zero without triggering a rube goldberg machine of coercions.
 In lie_algebra_element.pyx, in class
StructureCoefficientsElement
methodcpdef _bracket_(self, right)
, why not switch the next two statements:+ prod_c1_c2 = c1 * c2 + if not c2: + pass
Actually, is pass
the right thing here, rather than continue
? I thought pass
is just a nop; or is this a Cython oddity? (Same for the if not c1
a few lines earlier.)
comment:116 Changed 5 years ago by
 Commit changed from cf88fee19593c1dc07b7e11a64a1b7cfa1c0f148 to 42666d4c3685d9ec2d9851ae9618d3a9e809449f
Branch pushed to git repo; I updated commit sha1. New commits:
42666d4  Adding centralizer_basis method, improving to/from_vector for finitedim Lie algebras, other fixes.

comment:117 followup: ↓ 118 Changed 5 years ago by
Fixes all of comment:114 and comment:115 (the only bikeshedding could be in the is_zero
, but that might be a better approach, IDK exactly). Currently center
will not work until subalgebra
is implemented in generality, you can at least use L.centralizer_basis(L)
. The remaining TODO comments are the relevant ones.
comment:118 in reply to: ↑ 117 Changed 5 years ago by
Thanks for the changes. IMO the ticket is ready for a positive review. Darij, do you agree?
comment:119 Changed 5 years ago by
 Status changed from needs_review to needs_work
Building the pdf ref. manual with ./sage docbuild reference/categories pdf
ends with:
! Extra }, or forgotten $. l.29077 and recall that \(\mathfrak{g}_k} \supseteq \mathfrak{g}_{k+1}\).
Apparently, a {
is missing at the left of the first k
in line 560 of src/sage/categories/finite_dimensional_lie_algebras_with_basis.py
.
(sorry I haven't tried this sooner; on its side, ./sage docbuild reference/algebras pdf
is successfull).
comment:120 Changed 5 years ago by
 Commit changed from 42666d4c3685d9ec2d9851ae9618d3a9e809449f to 59df7ce401046c9bc1df6763d8ab80da594068a3
Branch pushed to git repo; I updated commit sha1. New commits:
59df7ce  Fixing pdf docbuild in finitedim Lie algebras w/ basis category.

comment:121 followup: ↓ 122 Changed 5 years ago by
 Status changed from needs_work to needs_review
Fixed the pdf docbuild.
comment:122 in reply to: ↑ 121 Changed 5 years ago by
comment:123 Changed 5 years ago by
Thanks for the fixes, Travis!
+ def product_space(self, L): + r""" + Return the product space ``[self, L]``. + + EXAMPLES:: + + sage: L = LieAlgebras(QQ).FiniteDimensional().WithBasis().example() + sage: a,b,c = L.lie_algebra_generators() + sage: X = L.subalgebra([a, b+c]) + sage: L.product_space(X) + An example of a finite dimensional Lie algebra with basis: + the 0dimensional abelian Lie algebra over Rational Field + with basis matrix: + [] + sage: Y = L.subalgebra([a, 2*bc]) + sage: X.product_space(Y) + An example of a finite dimensional Lie algebra with basis: + the 0dimensional abelian Lie algebra over Rational + Field with basis matrix: + [] + + :: + + sage: L.<x,y> = LieAlgebra(QQ, {('x','y'):{'x':1}}) + sage: Lp = L.product_space(L) # todo: not implemented  #17416 + sage: Lp # todo: not implemented  #17416 + Subalgebra generated of Lie algebra on 2 generators (x, y) over Rational Field with basis: + (x,) + sage: Lp.product_space(L) # todo: not implemented  #17416 + Subalgebra generated of Lie algebra on 2 generators (x, y) over Rational Field with basis: + (x,) + sage: L.product_space(Lp) # todo: not implemented  #17416 + Subalgebra generated of Lie algebra on 2 generators (x, y) over Rational Field with basis: + (x,) + sage: Lp.product_space(Lp) # todo: not implemented  #17416 + Subalgebra generated of Lie algebra on 2 generators (x, y) over Rational Field with basis: + () + """ + # Make sure we lift everything to the ambient space + try: + A = self._ambient + except AttributeError: + try: + A = L._ambient + except AttributeError: + A = self + + B = self.basis() + LB = L.basis() + K = B.keys() + LK = LB.keys() + # We echelonize the matrix here + # TODO: Do we want to? + b_mat = matrix(A.base_ring(), [A.bracket(B[a], LB[b]).to_vector() + for a in K for b in LK]) + b_mat.echelonize() + r = b_mat.rank() + I = A._basis_ordering + gens = [A.from_vector(row) for row in b_mat.rows()[:r]] + return A.subalgebra(gens)
I don't see why this should be a subalgebra (it is just a vector subspace). Counterexamples can easily be obtained if self (e.g.) is a free Lie algbera and L is the span of a single generator. (And that's just for the case when self and L both are Lie subalgebras; but the method is probably useful in higher generality.)
Why define I
if you don't use it? And why define K
and LK
if you could just iterate over B
and LB
?
As for echelonization... It will, of course, break over nonfields. But I'm not sure if this function is supposed to apply to nonfields.
comment:124 Changed 5 years ago by
So the current implementation is based upon returning the subalgebra generated by gens
, but indeed, this is not the right thing to do. So I should add something to check if L
(and self
) is an ideal or not. If it is, then the result is again an ideal.
As for nonfields, it should work (at least over nice enough rings):
sage: M = matrix([[1,2,5,2,3],[4,1,2,3,2],[6,2,3,1,2]]) sage: M.echelonize() sage: M [ 1 0 5 16 7] [ 0 1 0 7 2] [ 0 0 9 27 12] sage: M.base_ring() Integer Ring
comment:125 Changed 5 years ago by
Ah yes, it does work over the integers, though not over an arbitrary ring. Speaks in favor of an optional parameter.
I think there are good use cases for considering [U, V] when it is not a Lie ideal (let alone U and V). So maybe the result should really be just a subspace.
comment:126 Changed 5 years ago by
 Commit changed from 59df7ce401046c9bc1df6763d8ab80da594068a3 to ed587af6769afaae0dcd0ec9edd990f38c347cc0
Branch pushed to git repo; I updated commit sha1. New commits:
ed587af  Implementing ideal checks and fixing product_space.

comment:127 Changed 5 years ago by
I'm not sure about an optional parameter as that starts making things tricky because I think anything you would do likely has a call to echelonize
somewhere.
I agree completely that we should allow [U, V] to not just be a Lie ideal, and I have implemented that in the last commit.
comment:128 followup: ↓ 130 Changed 5 years ago by
Okay. Maybe worth explaining that thing with the base ring in the doc, though.
Something else:
+ # Do not override this. Instead implement :meth:`_construct_UEA`; + # then, :meth:`lift` and :meth:`universal_enveloping_algebra` + # will automatically setup the coercion
I might have already been nagging you about contracts and "what should I do if I want to set up a UEA for my own handmade Lie algebra" documentation. The above comment appears to explain it, but I'm not sure if it's enough. Literally it says I should just define _construct_UEA
and the rest will happen automatically. But don't I also need to define lift
, as it's otherwise just an abstract method in the base class? And is there a requirement that the base keys for the Lie algebra are a subset of the base keys for the UEA, or can a properly defined lift
method work around this?
comment:129 Changed 5 years ago by
 Commit changed from ed587af6769afaae0dcd0ec9edd990f38c347cc0 to c0f01df449f2e40c26d6e8002c42f08fec59e589
comment:130 in reply to: ↑ 128 Changed 5 years ago by
Replying to darij:
Okay. Maybe worth explaining that thing with the base ring in the doc, though.
It's better and easier to let it fail in the appropriate way and place. Errors do serve a useful purpose and going crazy in the doc doesn't help, especially when the base code is subject to change without notice.
Something else:
+ # Do not override this. Instead implement :meth:`_construct_UEA`; + # then, :meth:`lift` and :meth:`universal_enveloping_algebra` + # will automatically setup the coercionI might have already been nagging you about contracts and "what should I do if I want to set up a UEA for my own handmade Lie algebra" documentation. The above comment appears to explain it, but I'm not sure if it's enough. Literally it says I should just define
_construct_UEA
and the rest will happen automatically. But don't I also need to definelift
, as it's otherwise just an abstract method in the base class? And is there a requirement that the base keys for the Lie algebra are a subset of the base keys for the UEA, or can a properly definedlift
method work around this?
Answers to all of the above is no. It is actually more generic currently than I mentioned when we talked yesterday. It is clear from the code that one would need to define both _construct_UEA
and the lift
method for the elements (again, errors should result in the proper place(s)). However, I added a little more detail about this.
comment:131 Changed 5 years ago by
That clears things up, thanks!
There's a "the the" in the doc you just added. Aaaand no more nitpicking from me on this particular ticket :)
comment:132 Changed 5 years ago by
 Commit changed from c0f01df449f2e40c26d6e8002c42f08fec59e589 to fc395c8a0c03c608e2987b13dc0b8568e772046d
Branch pushed to git repo; I updated commit sha1. New commits:
fc395c8  Fixing typo.

comment:133 followup: ↓ 136 Changed 5 years ago by
 Status changed from needs_review to positive_review
I am going to interpret that as a positive review (since I fixed the "the the"). Thank you very much to both of you!!!
comment:134 Changed 5 years ago by
 Commit changed from fc395c8a0c03c608e2987b13dc0b8568e772046d to 64ab6054b1316ce9faad4ec7b71ed0c0a3440d38
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
64ab605  Trivial fix by making docstring a raw string.

comment:135 Changed 5 years ago by
 Status changed from needs_review to positive_review
Trivial change due to failing doctest from patchbot.
comment:136 in reply to: ↑ 133 Changed 5 years ago by
Replying to tscrim:
I am going to interpret that as a positive review (since I fixed the "the the"). Thank you very much to both of you!!!
Thanks and congratulations for your work. This is a great enhancement arriving in Sage!
comment:137 Changed 5 years ago by
 Status changed from positive_review to needs_work
Print order doesn't seem to be fixed, on OSX:
sage t long src/sage/algebras/lie_algebras/virasoro.py ********************************************************************** File "src/sage/algebras/lie_algebras/virasoro.py", line 418, in sage.algebras.lie_algebras.virasoro.VirasoroAlgebra.bracket_on_basis Failed example: d.bracket_on_basis(2, 2) Expected: 4*d[0]  1/2*c Got: 1/2*c  4*d[0] ********************************************************************** 1 item had failures: 1 of 4 in sage.algebras.lie_algebras.virasoro.VirasoroAlgebra.bracket_on_basis [59 tests, 1 failure, 1.51 s]
comment:138 Changed 5 years ago by
Could this be a general issue with the repr of combinatorial free modules whose basis is a DisjointUnionEnumeratedSets
?
In other news, it just caught my eye that the overridden is_ideal
on abelian Lie algebras tacitly assumes that the ambient Lie algebra of self
is abelian, too (otherwise, self
is not automatically a Lie ideal just by virtue of being abelian). I don't think this is clear to the user! I should have noticed this earlier :/ Travis, can you fix this or clarify my misunderstanding? Thank you!
comment:139 Changed 5 years ago by
 Commit changed from 64ab6054b1316ce9faad4ec7b71ed0c0a3440d38 to f8cb4764dce821a7076d3997a0345f6ce2443518
Branch pushed to git repo; I updated commit sha1. New commits:
f8cb476  Fix print order.

comment:140 Changed 5 years ago by
 Status changed from needs_work to needs_review
It is an issue of comparing a string to an integer. I added a key function that fixes the problem.
The abelian Lie algebra in the category examples folder, where it is just a toy implementation showing basis features? It is not meant to be a full implementation, just to provide an example of a basic implementation.
comment:141 Changed 5 years ago by
Ah yes, that one. Still would be better if it would raise a NotImplementedError? instead.
comment:142 Changed 5 years ago by
 Commit changed from f8cb4764dce821a7076d3997a0345f6ce2443518 to 98c488db05e40d8fd099673e8f8274febc2a8657
Branch pushed to git repo; I updated commit sha1. New commits:
98c488d  Must be more restrictive when checking ideals.

comment:143 Changed 5 years ago by
For the record, I disagree as it should be a minimal implementation. However, I have changed it so it punts it up to the category code (which is technically unnecessary now because afterwards I implemented it at the category level). This made me realize we needed to be more restrictive with the is_ideal
as it would run forever if given an infinite dimensional Lie algebra.
comment:144 Changed 5 years ago by
 Status changed from needs_review to positive_review
Nice job with the basis keys  back to positive review then!
comment:145 Changed 5 years ago by
Thank you!
comment:146 Changed 5 years ago by
 Branch changed from public/lie_algebras/fd_structure_coeff16820 to 98c488db05e40d8fd099673e8f8274febc2a8657
 Resolution set to fixed
 Status changed from positive_review to closed
Last 10 new commits:
Merge branch 'develop' into public/lie_algebras/fd_structure_coeff16820
Merge branch 'develop' into public/lie_algebras/fd_structure_coeff16820
Merge branch 'develop' into public/lie_algebras/fd_structure_coeff16820
Merge branch 'develop' into public/lie_algebras/fd_structure_coeff16820
Merge branch 'public/categories/lie_algebras16819' of trac.sagemath.org:sage into public/categories/lie_algebras16819
Created category for subalgebras of FD Lie algebras with basis.
Merge branch 'public/categories/lie_algebras16819' into public/lie_algebras/fd_structure_coeff16820
Added free_module abstract method.
Merge branch 'public/categories/lie_algebras16819' into public/lie_algebras/fd_structure_coeff16820
Many changes, fixes, and adding full doctest coverage.