Opened 8 years ago

Closed 6 years ago

# Implement ABCs and categories for Lie algebras and finite dimensional Lie algebras given by structure coefficients

Reported by: Owned by: Travis Scrimshaw Travis Scrimshaw major sage-8.0 algebra lie algebras, days64, sd67, days74, days79 Travis Scrimshaw Darij Grinberg, Eric Gourgoulhon N/A 98c488d 98c488db05e40d8fd099673e8f8274febc2a8657

### Description (last modified by Travis Scrimshaw)

Part of #14901. This implements the basic hierarchy of base classes, the categories for Lie algebras, some standard Lie algebras, and finite-dimensional Lie algebras given by structure coefficients.

### comment:1 Changed 8 years ago by Travis Scrimshaw

Branch: → public/lie_algebras/fd_structure_coeff-16820 → c2a3a982e419c0839f1fba68bf71b1014f243547 new → needs_review

Last 10 new commits:

 ​b35c8d3 Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​44a17b9 Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​59c0a96 Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​48ec188 Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​cbfbb87 Merge branch 'public/categories/lie_algebras-16819' of trac.sagemath.org:sage into public/categories/lie_algebras-16819 ​ed93f29 Created category for subalgebras of FD Lie algebras with basis. ​1a17ff9 Merge branch 'public/categories/lie_algebras-16819' into public/lie_algebras/fd_structure_coeff-16820 ​b302cc1 Added free_module abstract method. ​96083bf Merge branch 'public/categories/lie_algebras-16819' into public/lie_algebras/fd_structure_coeff-16820 ​c2a3a98 Many changes, fixes, and adding full doctest coverage.

### comment:2 Changed 8 years ago by git

Commit: c2a3a982e419c0839f1fba68bf71b1014f243547 → e5c1703bc6c958566a5f2a91d909e45b52b3c2b0

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

 ​c1f3eb3 Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​d0ddb8c Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​7424e75 Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​e5c1703 Fixed typos with sl2 construction.

### comment:3 Changed 8 years ago by git

Commit: e5c1703bc6c958566a5f2a91d909e45b52b3c2b0 → 87bba39ede0cfea37bb5e92b81aa851a2420863e

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

 ​45db323 Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​036995f Cleaning up/Fixing some logic, in particular for FromAssociative. ​a135fb3 Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​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 8 years ago by git

Commit: 87bba39ede0cfea37bb5e92b81aa851a2420863e → 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_algebras-16819' of trac.sagemath.org:sage into public/categories/lie_algebras-16819 ​980545e Fixed the doc. ​8a6950a Merge branch 'public/categories/lie_algebras-16819' 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_algebras-16819' into public/lie_algebras/fd_structure_coeff-16820 ​ece3cd9 Added files to the documentation.

### comment:5 Changed 8 years ago by git

Commit: ece3cd9d8ae826f28b82935d638de1f5df9d90a9 → 845e5aa14b7c97edeb04859fc92ebc63a78be1cb

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

 ​5197c7c Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​845e5aa Some last tweaks and getting full coverage.

### comment:6 Changed 8 years ago by Travis Scrimshaw

Keywords: lie algebras days64 added sage-6.4 → sage-6.6

### comment:7 Changed 8 years ago by git

Commit: 845e5aa14b7c97edeb04859fc92ebc63a78be1cb → 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_algebras-16819' into public/lie_algebras/fd_structure_coeff-16820 ​6ca7aca Fixing some parts of _construct_UEA. ​d35b756 Adding a default _basis_ordering and _dense_free_module ​0e33276 Merge branch 'public/categories/lie_algebras-16819' into public/lie_algebras/fd_structure_coeff-16820 ​e95d67c More fixes to fin-dim w/ basis category. ​c800d6c Merge branch 'public/categories/lie_algebras-16819' into public/lie_algebras/fd_structure_coeff-16820 ​a491bb0 Some more cleanup and fixes. ​aabf540 Fixing coefficient division. ​12074e8 Merge branch 'public/categories/lie_algebras-16819' into public/lie_algebras/fd_structure_coeff-16820

### comment:8 Changed 8 years ago by git

Commit: 12074e86295e2848a520c5962678c1c9e75b7505 → 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_algebras-16819' of git://trac.sagemath.org/sage into lie1 ​6201720 does this make sense? ​ddc77b1 Merge branch 'public/categories/lie_algebras-16819' 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_coeff-16820' of git://trac.sagemath.org/sage into lie2

### comment:9 Changed 8 years ago by git

Commit: 9830d63d19d1c4ec432a83ab8c21685ec7aae802 → 3d450840695003e8e410e0d0e3b926d7a00abe23

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

 ​6c5fe38 minor changes ​3d45084 fixing Jacobi identity for 3d example (unless Iam wrong? please add tests)

### comment:10 Changed 8 years ago by Darij Grinberg

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 8 years ago by git

Commit: 3d450840695003e8e410e0d0e3b926d7a00abe23 → 5e9a839618ae034d04c13340603a8b62d4bbf2f8

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

 ​d952a7b Merge branch 'public/categories/lie_algebras-16819' 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_coeff-16820' of git://trac.sagemath.org/sage into lie2 ​5e9a839 Jacobi identity now tested

### comment:12 Changed 8 years ago by git

Commit: 5e9a839618ae034d04c13340603a8b62d4bbf2f8 → 005984dfd1e329136db896a7dd29c1a2153dcff6

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

 ​005984d document the broken multiplication by base ring

### comment:13 Changed 8 years ago by git

Commit: 005984dfd1e329136db896a7dd29c1a2153dcff6 → 5bf7cf7b377985c6374e1a907f517cae62c063ea

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

 ​5bf7cf7 documenting LieAlgebraWithStructureCoefficients (mostly scaffolding for myself)

### comment:14 Changed 8 years ago by git

Commit: 5bf7cf7b377985c6374e1a907f517cae62c063ea → 4e8469da080ac7d303b74a8c697c6687b28e4dc3

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

 ​4e8469d fixing own bugs, and heeding pyflakes

### comment:15 Changed 8 years ago by git

Commit: 4e8469da080ac7d303b74a8c697c6687b28e4dc3 → 878ab1d0ed5dc2b0e27f1257caccb3747009bfb1

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

 ​878ab1d make generators work correctly for rank0 Heisenberg algebra

### comment:16 Changed 8 years ago by git

Commit: 878ab1d0ed5dc2b0e27f1257caccb3747009bfb1 → 6f6c529b753631bde338b7225968619e193db794

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

 ​6f6c529 p-Witt algebras, so that we have a characteristic-p example

### comment:17 Changed 8 years ago by git

Commit: 6f6c529b753631bde338b7225968619e193db794 → 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 3-dim 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 8 years ago by git

Commit: 052721a72568247cafc5abff310ad3478f999961 → 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 8 years ago by git

Commit: 4dd721a971b8606497cc046cf19e2706666cdf37 → 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 8 years ago by Darij Grinberg

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 8 years ago by git

Commit: ee3edc49855a1dd98bca0fad32f38897c5d9a3df → eb81ce8a62555f2157475b4190190feaaaa86e6a

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

 ​eb81ce8 document brokenness

### comment:22 Changed 8 years ago by git

Commit: eb81ce8a62555f2157475b4190190feaaaa86e6a → 87a227f1248a94beabd834007d458a505e753e46

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

 ​e37e0c7 trac #16820 oops, forgot one dot ​87a227f Merge branch 'public/lie_algebras/fd_structure_coeff-16820' with dot removal

### comment:23 follow-up:  25 Changed 8 years ago by Travis Scrimshaw

It at least covers all 3-dim 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 8 years ago by git

Commit: 87a227f1248a94beabd834007d458a505e753e46 → 8c5361b202dd68b31be6eb414b437badceb3fa76

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

 ​8c5361b meanwhile, fix _acted_upon_ bug

### comment:25 in reply to:  23 ; follow-up:  27 Changed 8 years ago by Darij Grinberg

Milestone: sage-6.6 → sage-6.7

It at least covers all 3-dim 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 try-except 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 a lift 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. A lift that can go anywhere depending on the concrete algebra will be rather useless.

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.

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.

Last edited 8 years ago by Darij Grinberg (previous) (diff)

### comment:26 Changed 8 years ago by git

Commit: 8c5361b202dd68b31be6eb414b437badceb3fa76 → 021ed1e344f8a30bc08407ac9069cb950429ff6d

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

 ​021ed1e more tests

### comment:27 in reply to:  25 ; follow-up:  29 Changed 8 years ago by Travis Scrimshaw

It at least covers all 3-dim 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:
pass


Does this do something different or is it just more pythonic? I am worried about this try-except 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 a lift 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. A lift 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 well-defined to me.

### comment:28 Changed 8 years ago by git

Commit: 021ed1e344f8a30bc08407ac9069cb950429ff6d → c1838221b369f47e407ffcd95fd53bd4f239e21d

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

 ​c183822 all doctests pass again :)

### comment:29 in reply to:  27 Changed 8 years ago by Darij Grinberg

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 a lift 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 well-defined to me.

0-dimensional 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 :)
Last edited 8 years ago by Darij Grinberg (previous) (diff)

### comment:30 Changed 8 years ago by Travis Scrimshaw

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 8 years ago by Darij Grinberg

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 upper-triangular.

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 8 years ago by git

Commit: c1838221b369f47e407ffcd95fd53bd4f239e21d → 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_actions-18221' of git://trac.sagemath.org/sage into lie2 ​c75e37b with trac 18221 merged in, some stopgaps removed

### comment:33 Changed 8 years ago by git

Commit: c75e37bbdc5a9208f04bd4b09a1f049aacf0bb32 → 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 8 years ago by Darij Grinberg

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, nc-relations: {}

::

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.

Last edited 8 years ago by Darij Grinberg (previous) (diff)

### comment:35 Changed 8 years ago by git

Commit: 3032fc09f15c90b61a3cf9b146e90b000c15fa4c → 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 8 years ago by git

Commit: bf2eb372d2135972a1e646d1120772da33dc8013 → 09281457c8cf34a075be3b72ae88296a75c54d93

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

 ​0928145 trivial edit

### comment:37 follow-up:  38 Changed 8 years ago by Travis Scrimshaw

Dependencies: #16819

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 ; follow-up:  39 Changed 8 years ago by Darij Grinberg

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.

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 one-element 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 that lift_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...

Last edited 8 years ago by Darij Grinberg (previous) (diff)

### comment:39 in reply to:  38 Changed 8 years ago by Travis Scrimshaw

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 one-element 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.

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 that lift_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 gln = sln 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 8 years ago by git

Commit: 09281457c8cf34a075be3b72ae88296a75c54d93 → 301ddb58fcbe191708c915f11985c37b73667922

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

 ​4148fc2 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' of git://trac.sagemath.org/sage into lie ​301ddb5 updating some doctests to recent changes in permutation groups (I guess)

### comment:41 Changed 8 years ago by git

Commit: 301ddb58fcbe191708c915f11985c37b73667922 → 690e2f50e7d4d0b7446738a3a60bd0faa8bba540

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

 ​690e2f5 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' into 6.8.b0

### comment:42 Changed 7 years ago by Frédéric Chapoton

Status: needs_review → needs_work

one failing doctest, see patchbot report

### comment:43 Changed 7 years ago by git

Commit: 690e2f50e7d4d0b7446738a3a60bd0faa8bba540 → 7a768e82f1eb1ccacf04169f8aadcbb4505eaae8

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

 ​a7f9800 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' of git://trac.sagemath.org/sage into public/lie_algebras/fd_structure_coeff-16820 ​53aec59 Fixed trivial doctest failure. ​7a768e8 Reworking the UEA for LieAlgebraFromAssociative.

### comment:44 Changed 7 years ago by Travis Scrimshaw

Status: needs_work → 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 7 years ago by git

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

 ​95ccdfc Merge branch 'public/lie_algebras/fd_structure_coeff-16820' into 6.9.b4 ​2f6b1e4 trac #16820 correct syntax for input

### comment:46 Changed 7 years ago by Travis Scrimshaw

Whoops :p, thanks Frederic.

### comment:47 Changed 7 years ago by Frédéric Chapoton

Status: needs_review → needs_work

two failing doctests

### comment:48 Changed 7 years ago by Frédéric Chapoton

Milestone: sage-6.7 → sage-6.10

### comment:49 Changed 7 years ago by git

Commit: 2f6b1e40e53f0039e5786a9390bdad1c9d2a7217 → 7c3978711e1ab0950e720b0bd7ba6e18c185114c

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

 ​7c39787 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' of trac.sagemath.org:sage into public/lie_algebras/fd_structure_coeff-16820

### comment:50 Changed 7 years ago by git

Commit: 7c3978711e1ab0950e720b0bd7ba6e18c185114c → 15e231797c8028a56d66eb19b067df90f2822822

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

 ​15e2317 Fixing doctests due to output order.

### comment:51 Changed 7 years ago by git

Commit: 15e231797c8028a56d66eb19b067df90f2822822 → b38f9838d5ebfcd120587543d60333c46ad3865a

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

 ​b38f983 documentation improvements

### comment:52 Changed 7 years ago by git

Commit: b38f9838d5ebfcd120587543d60333c46ad3865a → 090d308cd47fb5dfec0acc935972af18e2164ded

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

 ​090d308 oops

### comment:53 Changed 7 years ago by Travis Scrimshaw

Dependencies: → #17035

### comment:54 Changed 7 years ago by git

Commit: 090d308cd47fb5dfec0acc935972af18e2164ded → eedbb63c2964f52cb513666e67d73b56adcb1752

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

 ​a84a4ba Fixes for LieAlgebraFromAssociative. ​eedbb63 review of examples.py

### comment:55 Changed 7 years ago by git

Commit: eedbb63c2964f52cb513666e67d73b56adcb1752 → 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 sage-6.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_wBasis-18066' of trac.sagemath.org:sage into public/lie_algebras/fd_structure_coeff-16820

### comment:56 Changed 7 years ago by Travis Scrimshaw

Dependencies: #17035 → #17035 #18411 #18066 → Darij Grinberg needs_work → needs_review

### comment:57 Changed 7 years ago by git

Commit: 60031718f3b44571e9f83593aa7673a5cbb997d3 → 10ce8ff4c84248795e99f807aac2772c33e03cd6

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

 ​10ce8ff Fixing some things from recent positive review/merges.

### comment:58 Changed 7 years ago by git

Commit: 10ce8ff4c84248795e99f807aac2772c33e03cd6 → cdb2e5b9476356aa9f91d372a51aa802921f0e62

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

 ​cdb2e5b review heisenberg.py

### comment:59 Changed 7 years ago by git

Commit: cdb2e5b9476356aa9f91d372a51aa802921f0e62 → eda8d898c413e6244f515d72f6cbdca4aa2a763e

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

 ​eda8d89 review virasoro.py

### comment:60 Changed 7 years ago by git

Commit: eda8d898c413e6244f515d72f6cbdca4aa2a763e → be4a02edb5f542e8bb988243cdd80d4ff33b384f

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

 ​0fc61b3 Adding coercions for the Heisenberg algebra. ​be4a02e Merge branch 'public/lie_algebras/fd_structure_coeff-16820' of trac.sagemath.org:sage into public/lie_algebras/fd_structure_coeff-16820

### comment:61 Changed 7 years ago by git

Commit: be4a02edb5f542e8bb988243cdd80d4ff33b384f → 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 7 years ago by Travis Scrimshaw

Dependencies: #17035 #18411 #18066 → #17035 #18411 #18066 #10672

### comment:63 Changed 7 years ago by git

Commit: b8be3f4298913132df18d3230186f93686561223 → 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 7 years ago by git

Commit: 6815f1c02c6a13cef96d542328d3248e06ca9fb9 → 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_compatibility-10672' into public/lie_algebras/fd_structure_coeff-16820 ​573e074 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' of trac.sagemath.org:sage into public/lie_algebras/fd_structure_coeff-16820

### comment:65 Changed 7 years ago by Frédéric Chapoton

Status: needs_review → needs_work Implement ABCs for Lie algebras and finite dimensional given by structure cofficients → Implement ABCs for Lie algebras and finite dimensional given by structure coefficients

doc does not build

### comment:66 Changed 7 years ago by git

Commit: 573e0748daeb844cb79792c08a5c851ea047e7f1 → 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 7 years ago by Travis Scrimshaw

Status: needs_work → needs_review

### comment:68 Changed 7 years ago by git

Commit: 3f9a4a19a30d6749ab046160bc43988958c9490f → b9930e6c4f3f099baf341b58dc81d50a00df4986

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

 ​b9930e6 some changes to standardization of names

### comment:69 Changed 7 years ago by Darij Grinberg

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 7 years ago by Travis Scrimshaw

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 7 years ago by Darij Grinberg

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 7 years ago by Travis Scrimshaw

However I'm fixing the input so that it mends the input to use the indexing set.

### comment:73 Changed 7 years ago by Travis Scrimshaw

...in structure_coefficients.py.

### comment:74 Changed 7 years ago by git

Commit: b9930e6c4f3f099baf341b58dc81d50a00df4986 → 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 7 years ago by Travis Scrimshaw

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 7 years ago by Frédéric Chapoton

Milestone: sage-6.10 → sage-7.0

two failing doctests, see patchbot report

### comment:77 Changed 7 years ago by git

Commit: 60f743a2d8f8d684e4e636d6bdf329fae9967cb2 → 50f4441019eeb080c7e700e2853f9b6aa564cf7c

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

 ​a02380f Merge branch 'public/lie_algebras/fd_structure_coeff-16820' in 7.0.rc1 ​50f4441 trac #16820 correct the spelling of Cartesian

### comment:78 Changed 7 years ago by Frédéric Chapoton

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 7 years ago by Jeroen Demeyer

Dependencies: #17035 #18411 #18066 #10672 sage-7.0 → sage-7.1 needs_review → 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#headings-of-sage-library-code-files.

### comment:80 Changed 7 years ago by Darij Grinberg

Reviewers: Darij Grinberg

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 7 years ago by Travis Scrimshaw

Reviewers: → 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 7 years ago by git

Commit: 50f4441019eeb080c7e700e2853f9b6aa564cf7c → f1514bd21164a21c8ff90adf91bb036605fc1d85

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

 ​f1514bd Merge branch 'public/lie_algebras/fd_structure_coeff-16820' into 7.3.b2

### comment:83 Changed 7 years ago by git

Commit: f1514bd21164a21c8ff90adf91bb036605fc1d85 → a016182b1fcb8dcb4b29840be98436dda8531464

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

 ​7d155ec Fixing trivial doctest failures. ​a016182 Implementing a better way to compare indices.

### comment:84 Changed 7 years ago by Travis Scrimshaw

Keywords: days74 added sage-7.1 → sage-7.3

### comment:85 Changed 6 years ago by Frédéric Chapoton

apparently, doc does not build.

### comment:86 Changed 6 years ago by git

Commit: a016182b1fcb8dcb4b29840be98436dda8531464 → 6fcb3726142ab68451acc682d523b264bc1e319f

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

 ​6fcb372 Travis' fix for documentation

### comment:87 Changed 6 years ago by Aram Dermenjian

Status: needs_work → 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 git

Commit: 6fcb3726142ab68451acc682d523b264bc1e319f → 00816c6f058fcc1603cd4d8fdb9db495841f8ea5

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

 ​00816c6 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' into 7.3.b7

### comment:89 Changed 6 years ago by Frédéric Chapoton

one doctest complains about deprecation of generator_cmp

### comment:90 Changed 6 years ago by git

Commit: 00816c6f058fcc1603cd4d8fdb9db495841f8ea5 → 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 6 years ago by git

Commit: d5e618b4517c8766181a276da80f4e34080f60b6 → e3133954dd66f164586e5528f5825bbedb6f9e3a

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

 ​e313395 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' in 7.3.b9

### comment:92 Changed 6 years ago by Frédéric Chapoton

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 6 years ago by Travis Scrimshaw

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 6 years ago by Darij Grinberg

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 6 years ago by git

Commit: e3133954dd66f164586e5528f5825bbedb6f9e3a → e6851853a6de6e65cefabcd27b7ec20bf5ecfb25

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

 ​224da75 Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​8c0f1ab Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​16e36a9 Refactor to use free modules for fin-dim Lie algebras w/ structure coeffs; a bit more work needed still. ​e685185 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' of trac.sagemath.org:sage into public/lie_algebras/fd_structure_coeff-16820

### comment:96 Changed 6 years ago by Travis Scrimshaw

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 6 years ago by git

Commit: e6851853a6de6e65cefabcd27b7ec20bf5ecfb25 → 9f33f5d2da42d3c8619a1690d655579884418f2c

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

 ​cd730d0 Fixing universal enveloping algebra code. ​0238567 Cythonizing the element class. ​9f33f5d Fixing TestSuite for infinite-dimensional abelian Lie algebras.

### comment:98 Changed 6 years ago by git

Commit: 9f33f5d2da42d3c8619a1690d655579884418f2c → 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 6 years ago by Travis Scrimshaw

Milestone: sage-7.3 → sage-7.4 needs_review → 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 infinite-dimensional 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 6 years ago by git

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

 ​91c38a3 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' of trac.sagemath.org:sage into public/lie_algebras/fd_structure_coeff-16820 ​2e6692c Doing some fixes and bringing it up to full coverage.

### comment:101 Changed 6 years ago by git

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

 ​af54ea2 Cleaning up some (currently) unused imports.

### comment:102 Changed 6 years ago by Travis Scrimshaw

Keywords: days79 added needs_work → needs_review

Okay, back and fully ready for review.

### comment:103 Changed 6 years ago by git

Commit: af54ea281be2afdd7ddcc47527900ae152c111d3 → 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 6 years ago by git

Commit: d333710a1c9322000b5ae338ca037d3beb647a3e → 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 6 years ago by Tomer Bauer

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.

Last edited 6 years ago by Tomer Bauer (previous) (diff)

### comment:106 Changed 6 years ago by Frédéric Chapoton

there is an xrange, and many .iteritems (not good for py3)

### comment:107 Changed 6 years ago by git

Commit: eff47f6b34d6b9e987912cf8236a895b77596413 → 965c70b1ef515c23bf6ca3be3ff6bf8a9d03c94b

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

 ​f2b7cf2 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' in 7.5.b6 ​965c70b trac 16820 no more xrange

### comment:108 Changed 6 years ago by git

Commit: 965c70b1ef515c23bf6ca3be3ff6bf8a9d03c94b → 7c92fd10fd6a4557c98e908d5d8051d57c9ac630

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

 ​e8c5256 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' in 7.5.rc1 ​7c92fd1 trac 16820 nonzero -> bool

### comment:109 Changed 6 years ago by git

Commit: 7c92fd10fd6a4557c98e908d5d8051d57c9ac630 → 7bda1a49c73c19c3b18eb9fb0b07ece5f6323f65

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

 ​a9d636c Merge branch 'public/lie_algebras/fd_structure_coeff-16820' in 7.6.b5 ​7bda1a4 trac 16820 get rid of some .iteritems in py files

### comment:110 follow-up:  113 Changed 6 years ago by Eric Gourgoulhon

Reviewers: Darij Grinberg → 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 and LieAlgebraMatrixWrapper 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 infinite-dimensional 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't dimension be added to ParentMethods 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 by gens?
• The patchbot is complaining about "Python 3 incompatible code" in lie_algebra_element.pyx, namely the use of iteritems. Has this not been fixed by from 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 on LieAlgebra.__classcall_private__ to dispatch to the actual subclass corresponding to the input? Couldn't you use a function LieAlgebra to do this, as we did for manifolds (cf. the function Manifold)? (possibly renaming the class LieAlgebra to e.g. LieAlgebraBase to avoid any confusion). A drawback of the current setting is that the inline documentation obtained from LieAlgebra? 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 to LieAlgebra (because of the __classcall_private__ mechanism) and therefore may confuse the user. If you want to keep LieAlgebra.__classcall_private__, a solution could be to set the INPUT section in the docstring of the LieAlgebra class.

### comment:111 Changed 6 years ago by Eric Gourgoulhon

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 6 years ago by git

Commit: 7bda1a49c73c19c3b18eb9fb0b07ece5f6323f65 → cf88fee19593c1dc07b7e11a64a1b7cfa1c0f148

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

 ​3e2c547 Merge branch 'public/lie_algebras/fd_structure_coeff-16820' of git://trac.sagemath.org/sage into public/lie_algebras/fd_structure_coeff-16820 ​cf88fee Changes for the reviewer and using standard copyright.

### comment:113 in reply to:  110 ; follow-up:  114 Changed 6 years ago by Travis Scrimshaw

Description: modified (diff) sage-7.4 → sage-8.0 Implement ABCs for Lie algebras and finite dimensional given by structure coefficients → Implement ABCs and categories for Lie algebras and finite dimensional Lie algebras given by structure coefficients

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 and LieAlgebraMatrixWrapper 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 infinite-dimensional 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't dimension be added to ParentMethods 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 by gens?

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 of iteritems. Has this not been fixed by from 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 on LieAlgebra.__classcall_private__ to dispatch to the actual subclass corresponding to the input? Couldn't you use a function LieAlgebra to do this, as we did for manifolds (cf. the function Manifold)? (possibly renaming the class LieAlgebra to e.g. LieAlgebraBase to avoid any confusion). A drawback of the current setting is that the inline documentation obtained from LieAlgebra? 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 keep LieAlgebra.__classcall_private__, a solution could be to set the INPUT section in the docstring of the LieAlgebra 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 class-level 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 6 years ago by Eric Gourgoulhon

• 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 from lie_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 to LieAlgebra (because of the __classcall_private__ mechanism) and therefore may confuse the user. If you want to keep LieAlgebra.__classcall_private__, a solution could be to set the INPUT section in the docstring of the LieAlgebra 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 class-level 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 6 years ago by Darij Grinberg

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 up-to-date?
• In lie_algebra_element.pyx, is self == 0 or y == 0 really the right thing? I thought we had is_zero() for testing zero without triggering a rube goldberg machine of coercions.
• In lie_algebra_element.pyx, in class StructureCoefficientsElement method cpdef _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.)

Last edited 6 years ago by Darij Grinberg (previous) (diff)

### comment:116 Changed 6 years ago by git

Commit: cf88fee19593c1dc07b7e11a64a1b7cfa1c0f148 → 42666d4c3685d9ec2d9851ae9618d3a9e809449f

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

 ​42666d4 Adding centralizer_basis method, improving to/from_vector for finite-dim Lie algebras, other fixes.

### comment:117 follow-up:  118 Changed 6 years ago by Travis Scrimshaw

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 6 years ago by Eric Gourgoulhon

Thanks for the changes. IMO the ticket is ready for a positive review. Darij, do you agree?

### comment:119 Changed 6 years ago by Eric Gourgoulhon

Status: needs_review → 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 6 years ago by git

Commit: 42666d4c3685d9ec2d9851ae9618d3a9e809449f → 59df7ce401046c9bc1df6763d8ab80da594068a3

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

 ​59df7ce Fixing pdf docbuild in finite-dim Lie algebras w/ basis category.

### comment:121 follow-up:  122 Changed 6 years ago by Travis Scrimshaw

Status: needs_work → needs_review

Fixed the pdf docbuild.

### comment:122 in reply to:  121 Changed 6 years ago by Eric Gourgoulhon

Fixed the pdf docbuild.

Thanks. I'm back in the positive review mood then.

### comment:123 Changed 6 years ago by Darij Grinberg

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 0-dimensional abelian Lie algebra over Rational Field
+                 with basis matrix:
+                []
+                sage: Y = L.subalgebra([a, 2*b-c])
+                sage: X.product_space(Y)
+                An example of a finite dimensional Lie algebra with basis:
+                 the 0-dimensional 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 non-fields. But I'm not sure if this function is supposed to apply to non-fields.

Last edited 6 years ago by Darij Grinberg (previous) (diff)

### comment:124 Changed 6 years ago by Travis Scrimshaw

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 non-fields, 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 6 years ago by Darij Grinberg

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 6 years ago by git

Commit: 59df7ce401046c9bc1df6763d8ab80da594068a3 → ed587af6769afaae0dcd0ec9edd990f38c347cc0

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

 ​ed587af Implementing ideal checks and fixing product_space.

### comment:127 Changed 6 years ago by Travis Scrimshaw

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 follow-up:  130 Changed 6 years ago by Darij Grinberg

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 6 years ago by git

Commit: ed587af6769afaae0dcd0ec9edd990f38c347cc0 → c0f01df449f2e40c26d6e8002c42f08fec59e589

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

 ​3baf33f Merge branch 'develop' into public/lie_algebras/fd_structure_coeff-16820 ​c0f01df Cache is_ideal and explaining doc what is clear from the code.

### comment:130 in reply to:  128 Changed 6 years ago by Travis Scrimshaw

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 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?

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 6 years ago by Darij Grinberg

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 :)

Last edited 6 years ago by Darij Grinberg (previous) (diff)

### comment:132 Changed 6 years ago by git

Commit: c0f01df449f2e40c26d6e8002c42f08fec59e589 → fc395c8a0c03c608e2987b13dc0b8568e772046d

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

 ​fc395c8 Fixing typo.

### comment:133 follow-up:  136 Changed 6 years ago by Travis Scrimshaw

Status: needs_review → 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 6 years ago by git

Commit: fc395c8a0c03c608e2987b13dc0b8568e772046d → 64ab6054b1316ce9faad4ec7b71ed0c0a3440d38 positive_review → 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 6 years ago by Travis Scrimshaw

Status: needs_review → positive_review

Trivial change due to failing doctest from patchbot.

### comment:136 in reply to:  133 Changed 6 years ago by Eric Gourgoulhon

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 6 years ago by Volker Braun

Status: positive_review → 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 6 years ago by Darij Grinberg

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 6 years ago by git

Commit: 64ab6054b1316ce9faad4ec7b71ed0c0a3440d38 → f8cb4764dce821a7076d3997a0345f6ce2443518

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

 ​f8cb476 Fix print order.

### comment:140 Changed 6 years ago by Travis Scrimshaw

Status: needs_work → 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 6 years ago by Darij Grinberg

Ah yes, that one. Still would be better if it would raise a NotImplementedError? instead.

### comment:142 Changed 6 years ago by git

Commit: f8cb4764dce821a7076d3997a0345f6ce2443518 → 98c488db05e40d8fd099673e8f8274febc2a8657

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

 ​98c488d Must be more restrictive when checking ideals.

### comment:143 Changed 6 years ago by Travis Scrimshaw

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 6 years ago by Darij Grinberg

Status: needs_review → positive_review

Nice job with the basis keys -- back to positive review then!

Thank you!

### comment:146 Changed 6 years ago by Volker Braun

Branch: public/lie_algebras/fd_structure_coeff-16820 → 98c488db05e40d8fd099673e8f8274febc2a8657 → fixed positive_review → closed
Note: See TracTickets for help on using tickets.