Closed miguelmarco closed 9 years ago
Okay here's my first big pass of review. I've reworked a bunch of things:
CGAlgebra
.GCAlgebra
and CDGAlgebra
.TestSuite
checks (which led to #17224).over {base_ring}
last for CGAlgebra
to better follow the rest of Sage.There will be more changes and improvements coming, but I figured I should post what I have currently. In particular, I'm going to tweaks things around so that the differential specified for CDGAlgebra
acts naturally on its elements rather than using the coercions. I'm also going to implement a method to create a differential morphism, in case someone has a use for that independently.
Actually, that makes me ask this question. Should we put all of the chain complex implementations in the differentials? If so, should we have (decorator) class of CDGAlgebra
call those functions (which would mainly be a shorthand) or remove that class all together?
New commits:
cd59dbc | Merge branch 'u/jhpalmieri/DGA_new' of trac.sagemath.org:sage into public/algebras/cdga-16508 |
619624e | Merge branch 'u/jhpalmieri/DGA_new' of trac.sagemath.org:sage into public/algebras/cdga-16508 |
1b95a7e | First pass of reviewer changes: Various minor to moderate fixes. |
7dd57c5 | Some more minor tweaks. |
13a552e | Fix for pickling NC rings with weighted term order. |
9aaf591 | Merge branch 'public/pickling/term_order_NC_plural-17224' into public/algebras/cdga-16508 |
aae8f5b | Merge branch 'u/jhpalmieri/DGA_new' of trac.sagemath.org:sage into public/algebras/cdga-16508 |
7b6f118 | Fixed doctests. |
Dependencies: #17224
Changed branch from u/jhpalmieri/DGA_new to public/algebras/cdga-16508
Changed author from mmarco to Miguel Marco, John Palmieri
Replying to @tscrim:
Actually, that makes me ask this question. Should we put all of the chain complex implementations in the differentials? If so, should we have (decorator) class of
CDGAlgebra
call those functions (which would mainly be a shorthand) or remove that class all together?
What syntax are you imagining? A.differential().cohomology()
? Something like this would not be discoverable by tab completion. I think it makes a lot of sense to have a class for commutative DG algebras, and for instances of those, you should be able to call B.cohomology()
, etc.
Replying to @jhpalmieri:
What syntax are you imagining?
A.differential().cohomology()
? Something like this would not be discoverable by tab completion. I think it makes a lot of sense to have a class for commutative DG algebras, and for instances of those, you should be able to callB.cohomology()
, etc.
That's no more tab completable than A.CDGAlgebra().cohomology()
. The semantic would be d = A.differential({some: data})
. However I'm not saying that removing the CDGAlgebra
class is the best or right thing to do based on my current data/ideas. Yet I'm thinking of putting the actually implementations in the differential class and having CDGAlgebra
's corresponding methods call the corresponding differential's method in case we want to remove the class. IMO those methods fits naturally with the differential as well.
From my point of view, doing B=A.CDGAlgebra(...)
and then manipulating B
feels like the natural thing: I view the differential graded algebra as the central object of study here. Doing instead d=A.differential(...)
and then manipulating d
would not be the obvious thing to do at all.
So as long as in this situation, B.cohomology(...)
(etc.) works, I don't think I care about how it's implemented.
Replying to @jhpalmieri:
From my point of view, doing
B=A.CDGAlgebra(...)
and then manipulatingB
feels like the natural thing: I view the differential graded algebra as the central object of study here. Doing insteadd=A.differential(...)
and then manipulatingd
would not be the obvious thing to do at all.So as long as in this situation,
B.cohomology(...)
(etc.) works, I don't think I care about how it's implemented.
Now we can do both. I've also made the other changes I stated. There's one last thing, there is the method CDGAlgebra
and the class CDGAlgebra
, and if things are reordered in the module, you get failures from a name conflict. So the question is do we want to rename the method and fully spell that out to commutative_differential_graded_algebra
and/or the shorter cdg_algebra
, or do we leave it as it is? I'm strongly in favor of renaming the method to something else.
Branch pushed to git repo; I updated commit sha1. New commits:
d732dbe | Commutative DGAs: rename some classes |
I've made a few changes: maybe most crucially but also most trivially, there was a missing EXAMPLES::
that was causing docbuilding to fail.
Then I renamed some classes. The one I feel most strongly about is DifferentialCGA
: to me, this sounds like a "differential commutative graded algebra", not a "differential defined on a commutative graded algebra". So I just renamed it to Differential
. I changed CDGAlgebra
to DifferentialGCAlgebra
. (I toyed with GCAlgebraWithDifferential
, but didn't like it as much. I don't care that much, since these class names won't be that visible to most users.)
I also added a bit to the docstrings for the two CDGAlgebra
methods.
Branch pushed to git repo; I updated commit sha1. New commits:
497d9d6 | Merge branch 'public/algebras/cdga-16508' of trac.sagemath.org:sage into public/algebras/cdga-16508 |
Branch pushed to git repo; I updated commit sha1. New commits:
3900c66 | Renamed CDGAlgebra -> cdg_algebra. |
Replying to @jhpalmieri:
I've made a few changes: maybe most crucially but also most trivially, there was a missing
EXAMPLES::
that was causing docbuilding to fail.
Whoops.
Then I renamed some classes. The one I feel most strongly about is
DifferentialCGA
: to me, this sounds like a "differential commutative graded algebra", not a "differential defined on a commutative graded algebra". So I just renamed it toDifferential
. I changedCDGAlgebra
toDifferentialGCAlgebra
. (I toyed withGCAlgebraWithDifferential
, but didn't like it as much. I don't care that much, since these class names won't be that visible to most users.)
I'm happy with this name change (it gets the association from the module name). However I've rename the method CDGAlgebra
to cdg_algbra
match conventions.
I also added a bit to the docstrings for the two
CDGAlgebra
methods.
Looks good. So if you're happy with my changes, then I think we can set a positive review.
Travis, should you be listed as an author in the file commutative_dga.py
?
I don't consider myself as an author since I mostly just reorganized code and did touch-ups.
Changed branch from public/algebras/cdga-16508 to 3900c66
See #21845 as a followup: cohomology algebra generators.
Changed commit from 3900c66
to none
Changed keywords from sd58, sd59, algebras, nonconmutative, graded to sd58, sd59, algebras, noncommutative, graded
This patch adds basic CDGA's.
They work as follows, first you create the algebra:
Then define the differential on
A
to construct a differential graded algebra:Now you can compute things like a basis of each homogeneous part:
Or the cohomology in each degree:
Depends on #17224
CC: @vbraun @simon-king-jena @saraedum @tscrim
Component: algebra
Keywords: sd58, sd59, algebras, noncommutative, graded
Author: Miguel Marco, John Palmieri
Branch:
3900c66
Reviewer: Travis Scrimshaw
Issue created by migration from https://trac.sagemath.org/ticket/16508