Opened 3 years ago

Closed 3 years ago

#28149 closed enhancement (fixed)

Implement compact real forms for matrix Lie algebras

Reported by: tscrim Owned by:
Priority: major Milestone: sage-8.9
Component: algebra Keywords: lie algebras, fpsac2019
Cc: bsalisbury1, chapoton Merged in:
Authors: Travis Scrimshaw Reviewers: Ben Salisbury, Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: 4adf53b (Commits, GitHub, GitLab) Commit: 4adf53b410b178cfc39b24678af9f579a8b2dc40
Dependencies: Stopgaps:

Status badges

Description

Because they are interesting and are useful.

Change History (17)

comment:1 Changed 3 years ago by tscrim

  • Branch set to public/lie_algebras/compact_real_form-28149
  • Commit set to 1fb62d7693b3d3a4084be40c6f9a36ce6af9e85c
  • Status changed from new to needs_review

New commits:

1fb62d7Implementation of compact real forms for classical matrix Lie algebras.

comment:2 Changed 3 years ago by git

  • Commit changed from 1fb62d7693b3d3a4084be40c6f9a36ce6af9e85c to 23f093076a47c77127c326a697c2db4dec3b1b23

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

23f0930Added su to Lie algebra examples.

comment:3 Changed 3 years ago by git

  • Commit changed from 23f093076a47c77127c326a697c2db4dec3b1b23 to e72b7c92e88f16212f8f7bb347ea2e4ca37e3b75

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

e72b7c9Doing some more documentation and small fixes.

comment:4 Changed 3 years ago by bsalisbury1

  • Cc bsalisbury1 added

comment:5 Changed 3 years ago by git

  • Commit changed from e72b7c92e88f16212f8f7bb347ea2e4ca37e3b75 to 429e156c0180e7dc87a7d9ef4b76ca53cdb45466

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

0bea719Merge branch 'develop' into t/28149/public/lie_algebras/compact_real_form-28149
429e156editorial changes

comment:6 Changed 3 years ago by bsalisbury1

  • Reviewers set to Ben Salisbury

Hi Travis,

I made some small changes to the documentation. If you are okay with these changes, you may set this to positive review on my behalf.

~Ben

comment:7 Changed 3 years ago by tscrim

  • Status changed from needs_review to positive_review

Thank you!

comment:8 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

On Python 3:

sage -t --long src/sage/algebras/lie_algebras/classical_lie_algebra.py
**********************************************************************
File "src/sage/algebras/lie_algebras/classical_lie_algebra.py", line 1202, in sage.algebras.lie_algebras.classical_lie_algebra.MatrixCompactRealForm.Element._richcmp_
Failed example:
    sorted(L.basis())
Exception raised:
    Traceback (most recent call last):
      File "/var/lib/buildbot/slave/sage3_git/build/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 681, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/var/lib/buildbot/slave/sage3_git/build/local/lib/python3.7/site-packages/sage/doctest/forker.py", line 1105, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.algebras.lie_algebras.classical_lie_algebra.MatrixCompactRealForm.Element._richcmp_[1]>", line 1, in <module>
        sorted(L.basis())
      File "sage/structure/element.pyx", line 1091, in sage.structure.element.Element.__richcmp__ (build/cythonized/sage/structure/element.c:9923)
        return (<Element>self)._richcmp_(other, op)
      File "sage/structure/element.pyx", line 1095, in sage.structure.element.Element._richcmp_ (build/cythonized/sage/structure/element.c:10030)
        cpdef _richcmp_(left, right, int op):
      File "/var/lib/buildbot/slave/sage3_git/build/local/lib/python3.7/site-packages/sage/algebras/lie_algebras/classical_lie_algebra.py", line 1208, in _richcmp_
        return richcmp((self._real, self._imag), (other._real, other._imag), op)
      File "sage/structure/richcmp.pxd", line 6, in sage.structure.richcmp.richcmp (build/cythonized/sage/structure/richcmp.c:3043)
        cpdef inline richcmp(x, y, int op):
      File "sage/structure/richcmp.pxd", line 50, in sage.structure.richcmp.richcmp (build/cythonized/sage/structure/richcmp.c:2939)
        return PyObject_RichCompare(x, y, op)
      File "sage/structure/element.pyx", line 1091, in sage.structure.element.Element.__richcmp__ (build/cythonized/sage/structure/element.c:9923)
        return (<Element>self)._richcmp_(other, op)
      File "sage/matrix/matrix_sparse.pyx", line 373, in sage.matrix.matrix_sparse.Matrix_sparse._richcmp_ (build/cythonized/sage/matrix/matrix_sparse.c:5799)
        return richcmp(self._dict(), right._dict(), op)
      File "sage/structure/richcmp.pxd", line 50, in sage.structure.richcmp.richcmp (build/cythonized/sage/matrix/matrix_sparse.c:14822)
        return PyObject_RichCompare(x, y, op)
    TypeError: '<' not supported between instances of 'dict' and 'dict'
**********************************************************************
1 item had failures:
   1 of   3 in sage.algebras.lie_algebras.classical_lie_algebra.MatrixCompactRealForm.Element._richcmp_
    [210 tests, 1 failure, 47.86 s]

comment:9 Changed 3 years ago by git

  • Commit changed from 429e156c0180e7dc87a7d9ef4b76ca53cdb45466 to bf3c240e84afd7c53672afa7401d7e20e2d72d9f

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

bf3c240More explicit comparisons for sparse matrices (for Python3).

comment:10 Changed 3 years ago by tscrim

  • Cc chapoton added
  • Status changed from needs_work to needs_review

The root cause is that dict's are incomparable on Python3, which means sparse matrices are incomparable, unlike dense matrices. So I changed how comparisons are done for sparse matrices, which is why I put you in cc Frédéric.

comment:11 Changed 3 years ago by git

  • Commit changed from bf3c240e84afd7c53672afa7401d7e20e2d72d9f to 8afd478fbbcac4e7763a4f2e73ce2a0c8571cb2e

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

8afd478Fixing last doctest.

comment:12 Changed 3 years ago by tscrim

(Now) Passes all doctests. Can someone review the last two changes? Should be quick.

comment:13 follow-up: Changed 3 years ago by chapoton

  • some pyflakes warnings, see last patchbot (only 2 fixable ones)
  • typo "tranposes"
  • here the brackets can be removed, I think:

+ names = tuple(['CR%s'%s for s in range(dim)])

Otherwise, looks good. You can set to positive once the above is fixed.

comment:14 Changed 3 years ago by chapoton

  • Reviewers changed from Ben Salisbury to Ben Salisbury, Frédéric Chapoton

comment:15 Changed 3 years ago by git

  • Commit changed from 8afd478fbbcac4e7763a4f2e73ce2a0c8571cb2e to 4adf53b410b178cfc39b24678af9f579a8b2dc40

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

4adf53bFinal tidbits from Frederic.

comment:16 in reply to: ↑ 13 Changed 3 years ago by tscrim

  • Status changed from needs_review to positive_review

Replying to chapoton:

  • some pyflakes warnings, see last patchbot (only 2 fixable ones)

Fixed.

  • typo "tranposes"

Fixed.

  • here the brackets can be removed, I think:

+ names = tuple(['CR%s'%s for s in range(dim)])

It is actually slightly faster to have the brackets. I think internally Python has some optimization for constructing tuples from lists.

Otherwise, looks good. You can set to positive once the above is fixed.

Thank you. I am allowing myself to set this to a positive review.

comment:17 Changed 3 years ago by vbraun

  • Branch changed from public/lie_algebras/compact_real_form-28149 to 4adf53b410b178cfc39b24678af9f579a8b2dc40
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.