-
-
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
Subcategories and base class for nilpotent Lie algebras #26074
Comments
comment:1
An initial working copy of the implementation is in the commits. Whether the category framework was used correctly I do not know. In particular the distinction between axioms like Currently, based on my understanding of http://doc.sagemath.org/html/en/reference/categories/sage/categories/primer.html#difference-between-axioms-and-regressive-covariant-functorial-constructions, I added Moreover, the examples are currently slightly hand-built (e.g. by forcing an extra category on a nilpotent Lie algebra constructor). The main issue that I do not how to solve is that even in the case when the e.g. the natural stratification can be deduced (as in the implementation of the Additionally, as a minor tweak I added an extra parameter |
comment:2
So Most of your implementation of the categories is good on a quick look-over. There are a few things that could be simplified that I can work on tomorrow to make the structures a little cleaner given the code that is currently in the categories. We can easily expand things out later if there is a use for them. I think the easiest way to explain things will be me making some commits with the changes with some comments here, but if you would rather me just give comments, then let me know. As to realizing the correct category, there is a
I should caution that this has been known to cause issues in the past, but mostly because these get used as the base ring (and hence, parameterize the category) for other objects. Although for general purpose classes, I am inclined to force the user to ask for such a property (such as in the example above), which can include calling particular methods. +1 for the added option to As previously mentioned, I will take a detailed look tomorrow. I am assuming I can otherwise treat the code as needs review. |
comment:3
Feel free to make any changes, I agree that it is much easier to understand things with something concrete to look at. The Your final assumption is correct, other than my confusion about the categories, I would have classified the code as 'needs review'. |
comment:4
I have made it through and reworked the categories around a bit, but it is more in line with what should be done (at least as I have learned it). As some extra test cases, I made the Heisenberg Lie algebra(s) in the nilpotent category. I have done a few other tweaks to the doc and code, including fixing some other cases for Hopefully these changes (currently) will not cause you too many refactoring problems with later tickets. New commits:
|
Changed branch from u/gh-ehaka/nilpotent_lie_algebras to public/lie_algebras/base_class_nilponent-26074 |
Changed author from Eero Hakavuori to Eero Hakavuori, Travis Scrimshaw |
Reviewer: Travis Scrimshaw, Eero Hakavuori |
comment:5
Thanks for the improvements, looks quite good to me. Probably you are right about the I noticed in the edits the
whereas mathematically this should be True. Then again this does not really depend on the basis, so if I understand correctly, the right place to add the Regarding the text presentation of the categories, the automatic printing is a bit cumbersome:
It makes sense to have |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:7
I removed the I added the 'finite dimensional + stratified => nilpotent' deduction to
and it didn't break the existing deduction
so I assume adding the I also fixed some doctests in |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:10
Replying to @ehaka:
Thank you. Looks good. I added an extra option so that when given
Sorry about loosing that. Yes, this is the correct way to do it.
I fixed two others that were coming back from the patchbot. I know the text representation of the category is not very good. I will see if I can make it better (I think you can, but I need to dig around a little and experiment), but beyond that, I am happy with things here. |
comment:11
The edits all look good to me. I would be ok with marking this as positive review and handling the category repr improvements eventually in a later ticket as it is only a cosmetic fix. |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:13
Here is the improvements to the category repr. The "stratified graded" -> "stratified" was a standard(ish) workaround. Removing the "nilpotent" was a bit more work, but mainly because we do not want to say "nilpotent finite dimensional Lie algebras". However, it falls under the same workaround as the other. So if these last changes are good, then positive review. |
comment:14
Replying to @tscrim:
Very nice! The various combinations of finite dimensional, graded, stratified and nilpotent all printed quite well now in my opinion. |
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
|
comment:16
Docbuild was failing because of the non-existent module I'll leave the ticket as needs review until it is verified that the doc changes were correct as I have not tried to modify the automatic html reference before. |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:18
Yep, LGTM. Thank you for catching and fixing that. I made some trivial changes and am allowing myself to set this to a positive review (I changed the Vk to Lk to be consistent notationally; feel free to revert the positive review if you disagree). |
Changed branch from public/lie_algebras/base_class_nilponent-26074 to |
Implementation of subcategories for nilpotent, graded and stratified Lie algebras, and a base class and constructor for nilpotent Lie algebras mimicking
LieAlgebraWithStructureCoefficients
.CC: @tscrim
Component: categories
Keywords: Lie algebras, nilpotent, graded, stratified
Author: Eero Hakavuori, Travis Scrimshaw
Branch/Commit:
120c02a
Reviewer: Travis Scrimshaw, Eero Hakavuori
Issue created by migration from https://trac.sagemath.org/ticket/26074
The text was updated successfully, but these errors were encountered: