Implement Chevalley-Eilenberg complexes with module coefficients #36171
Conversation
|
@adityadwarkesh Here is the code for the CE complex split off as a separate PR. I ended up finding it easier to rewrite the code starting from what I had, but I incorporated some of the tricks that you employed. |
… finite dimensional modules.
d9c21a9 to
7ee7aec
Compare
There was a problem hiding this comment.
A few remarks:
- The comment on line 1288 seems superfluous.
- Codecov mentions a number of lines not covered by tests, although I'm not sure how significant this is.
- When I try running this locally (in particular, computing the example offered), I'm receiving a "Typerror: type 'dict' is not hashable" error. I remember running into this last summer and somehow getting rid of it.
That's not quite true. It is important to distinguish between the two indices.
Codecov is just wrong here. It isn't smart enough to distinguish the internal function being called by the outer test. I wish there was a way to turn it off.
Without knowing what you are doing, I can't respond beyond saying it works for me and the doctests are passing. |
Right, my mistake—I should have said that line 1290 seems superfluous? It doesn't seem to refer to anything in particular.
OK.
OK, then I think we can conclude that it doesn't have to do with the code. (For what it's worth, it was working fine when I tried it on cocalc.) I think it seems good to go, then |
I thought it did, but looking at it again, you're right. Removed.
It would still be good to see what you're trying. There might be an issue there (with your usage, the code, or both) that I would like to check.
Great; thank you! |
|
Documentation preview for this PR (built with commit adbccdc; changes) is ready! 🎉 |
|
Thank you! |
sagemathgh-36171: Implement Chevalley-Eilenberg complexes with module coefficients <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> We implement the Chevalley-Eilenberg complex of a Lie algebra `L` with coefficients in a general `L` representation. <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [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 - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> - sagemath#36169 uses the representations of Lie algebras <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36171 Reported by: Travis Scrimshaw Reviewer(s): adityadwarkesh
We implement the Chevalley-Eilenberg complex of a Lie algebra
Lwith coefficients in a generalLrepresentation.📝 Checklist
⌛ Dependencies