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

[tscrim]

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.

[tscrim]

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.

[git]

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.

[git]

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.

[git]

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.

[git]

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.

[git]

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

[git]

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

[git]

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)

[darij]

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

[git]

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

[git]

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

​005984d

document the broken multiplication by base ring

[git]

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

​5bf7cf7

documenting LieAlgebraWithStructureCoefficients (mostly scaffolding for myself)

[git]

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

​4e8469d

fixing own bugs, and heeding pyflakes

[git]

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

​878ab1d

make generators work correctly for rank0 Heisenberg algebra

[git]

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

​6f6c529

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

[git]

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.

[git]

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

​4dd721a

trac #16820 fixing doc inclusion once again (no .py)

[git]

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

[darij]

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

[git]

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

​eb81ce8

document brokenness

[git]

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

[tscrim]

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.

[git]

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

​8c5361b

meanwhile, fix _acted_upon_ bug

[darij]

Replying to tscrim:

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.

[git]

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

​021ed1e

more tests

[tscrim]

Replying to darij:

Replying to tscrim:

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.

[git]

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

​c183822

all doctests pass again :)

[darij]

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

[tscrim]

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.

[darij]

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

[git]

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

[git]

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

[darij]

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.

[git]

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

​bf2eb37

same optimization as in trac 17098, plus getting rid of todos

[git]

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

​0928145

trivial edit

[tscrim]

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?

[darij]

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

[tscrim]

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

[git]

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)

[git]

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

[chapoton]

one failing doctest, see patchbot report

[git]

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.

[tscrim]

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.

[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

[tscrim]

Whoops :p, thanks Frederic.

[chapoton]

two failing doctests

[git]

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

[git]

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

​15e2317

Fixing doctests due to output order.

[git]

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

​b38f983

documentation improvements

[git]

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

​090d308

oops

[git]

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

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

[git]

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

[git]

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

​10ce8ff

Fixing some things from recent positive review/merges.

[git]

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

​cdb2e5b

review heisenberg.py

[git]

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

​eda8d89

review virasoro.py

[git]

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

[git]

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

[git]

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

[git]

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

[chapoton]

doc does not build

[git]

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

​3f9a4a1

Fixing the failing doctests and making the doc build.

[git]

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

​b9930e6

some changes to standardization of names

[darij]

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

[tscrim]

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.

[darij]

Ah! You're right, this is user error. I got confused by the fact that the names come before the index_set.

[tscrim]

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

[tscrim]

...in structure_coefficients.py.

[git]

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

​60f743a

Parsing the input so that it uses the indexing set.

[tscrim]

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]

[chapoton]

two failing doctests, see patchbot report

[git]

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

[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.

[jdemeyer]

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.

[darij]

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

[tscrim]

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.

[git]

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

[git]

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

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

[chapoton]

apparently, doc does not build.

[git]

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

​6fcb372

Travis' fix for documentation

[aram.dermenjian]

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.

[git]

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

[chapoton]

one doctest complains about deprecation of generator_cmp

[git]

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

[git]

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

[chapoton]

Travis and Darij, do you think that this still needs a lot of work ? That would be great to have in sage !

[tscrim]

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.

[darij]

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

[git]

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

[tscrim]

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.

[git]

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.

[git]

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

​394c3f0

Not creating a new vector at every step in _bracket_.

[tscrim]

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.

[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.

[git]

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

​af54ea2

Cleaning up some (currently) unused imports.

[tscrim]

Okay, back and fully ready for review.

[git]

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

​d333710

Omission of abelian Lie algebras from doc index, and citation fix.

[git]

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

​eff47f6

standardize_names_index_set: Fix doctest and better documentation

[mathzeta2]

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.

[chapoton]

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

[git]

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

[git]

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

[git]

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

[egourgoulhon]

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.

[egourgoulhon]

Another minor remark: the ticket title looks strange to me: shouldn't a name ("Lie algebras", I guess) be inserted behind "finite dimensional"?

[git]

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.

[tscrim]

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

[egourgoulhon]

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

[darij]

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

[git]

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.

[tscrim]

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.

[egourgoulhon]

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

[egourgoulhon]

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

[git]

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

​59df7ce

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

[tscrim]

Fixed the pdf docbuild.

[egourgoulhon]

Replying to tscrim:

Fixed the pdf docbuild.

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

[darij]

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.

[tscrim]

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

[darij]

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.

[git]

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

​ed587af

Implementing ideal checks and fixing product_space.

[tscrim]

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.

[darij]

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?

[git]

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.

[tscrim]

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

[darij]

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

[git]

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

​fc395c8

Fixing typo.

[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!!!

[git]

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.

[tscrim]

Trivial change due to failing doctest from patchbot.

[egourgoulhon]

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!

[vbraun]

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]

[darij]

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!

[git]

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

​f8cb476

Fix print order.

[tscrim]

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.

[darij]

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

[git]

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

​98c488d

Must be more restrictive when checking ideals.

[tscrim]

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.

[darij]

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

[tscrim]

Thank you!