-
-
Notifications
You must be signed in to change notification settings - Fork 482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement the solvable and nil radicals of a finite dimensional Lie algebra #37697
Implement the solvable and nil radicals of a finite dimensional Lie algebra #37697
Conversation
83973fd
to
36b29b9
Compare
Test failures in |
d92aa8c
to
005a6c5
Compare
Documentation preview for this PR (built with commit abf746e; changes) is ready! 🎉 |
005a6c5
to
8be290b
Compare
""" | ||
if sparse: | ||
return self.value.sparse_vector() | ||
return self.value | ||
|
||
cpdef to_vector(self, bint sparse=False, order=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why can it just ignore the order
parameter?
(Of course I see that the original code did the same.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the implementation is based on a fixed order and it is added for (signature) compatibility with the generic implementation.
(b, c, d) | ||
|
||
sage: SL = L.subalgebra([a,b,c,d]); SL | ||
Subalgebra generated by (a, b, c, d) of Lie algebra on 5 generators (a, b, c, d, e) over Rational Field |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Subalgebra generated by (a, b, c, d) of Lie algebra on 5 generators (a, b, c, d, e) over Rational Field | |
Subalgebra generated by (a, b, c, d) of | |
Lie algebra on 5 generators (a, b, c, d, e) over Rational Field |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throughout I recommend to break long output at column 88 so that it fits in the formatted HTML doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this everywhere I felt it was at most a net-zero improvement in readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM
8be290b
to
abf746e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Let's ship it.
Thank you! |
… dimensional Lie algebra <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> We implement the algorithms in de Graaf to compute the solvable and nilradicals of a finite dimensional Lie algebra. To do so, we also make further improvements to how subalgebras and ideals of Lie algebras are constructed and handled. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> - sagemath#37693 Uses this in the computation - sagemath#37682 Needs the improvements here URL: sagemath#37697 Reported by: Travis Scrimshaw Reviewer(s): Matthias Köppe, Travis Scrimshaw
We implement the algorithms in de Graaf to compute the solvable and nilradicals of a finite dimensional Lie algebra. To do so, we also make further improvements to how subalgebras and ideals of Lie algebras are constructed and handled.
📝 Checklist
⌛ Dependencies