Changes between Initial Version and Version 113 of Ticket #16820


Ignore:
Timestamp:
03/27/17 23:09:37 (5 years ago)
Author:
tscrim
Comment:

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.

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #16820

    • Property Status changed from new to needs_review
    • Property Reviewers changed from to Darij Grinberg, Eric Gourgoulhon
    • Property Summary changed from Implement ABCs for Lie algebras and finite dimensional given by structure cofficients to Implement ABCs and categories for Lie algebras and finite dimensional Lie algebras given by structure coefficients
    • Property Dependencies changed from #16819 to
    • Property Branch changed from to public/lie_algebras/fd_structure_coeff-16820
    • Property Milestone changed from sage-6.4 to sage-8.0
    • Property Keywords lie algebras days64 sd67 days74 days79 added
    • Property Commit changed from to cf88fee19593c1dc07b7e11a64a1b7cfa1c0f148
  • Ticket #16820 – Description

    initial v113  
    1 Part of #14901. This will also implement the basic hierarchy of base classes.
     1Part 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.