Double complexes #2935
Double complexes #2935
Conversation
|
I added a first implementation for morphisms of complexes (not complexes of morphisms!), as was suggested by @wdecker . For the proof of concept I implemented a version which lifts a morphism of modules incrementally through their free resolutions. I will link it in the source code below. |
| res1 = free_resolution(M1).C | ||
| res2 = free_resolution(M2).C | ||
|
|
||
| psi = Oscar.lift_morphism_through_free_resolutions(phi, domain_resolution=res2, codomain_resolution=res1); |
There was a problem hiding this comment.
This is the command to lift a morphism of modules phi : M -> N to a morphism of their resolutions.
| function lift_morphism_through_free_resolutions(phi::ModuleFPHom; | ||
| domain_resolution::ComplexOfMorphisms=free_resolution(domain(phi)).C, | ||
| codomain_resolution::ComplexOfMorphisms=free_resolution(codomain(phi)).C | ||
| ) | ||
| factory = LiftingMorphismsThroughResolution(phi) | ||
|
|
||
| return MorphismOfComplexes(domain_resolution, codomain_resolution, factory, 0, | ||
| left_bound = -2, right_bound = -1, | ||
| extends_left = false, extends_right = true) | ||
| end |
There was a problem hiding this comment.
This is the actual implementation of that function's method. It involves creating a "factory" to do the lifting on request.
| mutable struct LiftingMorphismsThroughResolution{T} <: AbsMorphismFactory{T} | ||
| original_map::ModuleFPHom | ||
|
|
||
| function LiftingMorphismsThroughResolution(phi::ModuleFPHom) | ||
| return new{ModuleFPHom}(phi) | ||
| end | ||
| end |
There was a problem hiding this comment.
The "factory" is a concrete type which remembers the original morphism. The call signature of this struct will be overloaded to do the lifting.
| function (fac::LiftingMorphismsThroughResolution)(phi::AbsMorphismOfComplexes, i::Int) | ||
| i == -1 && return fac.original_map | ||
| i == -2 && return hom(domain(phi)[-2], codomain(phi)[-2], elem_type(codomain(phi)[-2])[]) # the zero morphism | ||
|
|
||
| # Build up the maps recursively via liftings | ||
| psi = phi[i-1] | ||
| dom_map = map(domain(phi), i) | ||
| cod_map = map(codomain(phi), i) | ||
| x = gens(domain(dom_map)) | ||
| y = psi.(dom_map.(x)) | ||
| z = [preimage(cod_map, u) for u in y] | ||
| return hom(domain(dom_map), domain(cod_map), z) | ||
| end |
There was a problem hiding this comment.
Here we see the lifting actually being computed.
HechtiDerLachs
force-pushed
the
double_complexes
branch
from
1033978
to
d8b6ca0
Compare
|
I added some user facing documentation. @wdecker @jankoboehm @thofma @fieker : Could you have a look and let me know whether you approve of this? More documentation for the programmer about the internals will follow. |
Codecov Report
@@ Coverage Diff @@
## master #2935 +/- ##
===========================================
+ Coverage 43.74% 80.20% +36.45%
===========================================
Files 458 481 +23
Lines 65036 67941 +2905
===========================================
+ Hits 28451 54492 +26041
+ Misses 36585 13449 -23136
|
| ```@docs | ||
| has_upper_bound(D::AbsDoubleComplexOfMorphisms) | ||
| has_lower_bound(D::AbsDoubleComplexOfMorphisms) | ||
| has_right_bound(D::AbsDoubleComplexOfMorphisms) | ||
| has_left_bound(D::AbsDoubleComplexOfMorphisms) | ||
| ``` | ||
| If they exist, these bounds can be asked for using | ||
| ```@docs | ||
| right_bound(D::AbsDoubleComplexOfMorphisms) | ||
| left_bound(D::AbsDoubleComplexOfMorphisms) | ||
| upper_bound(D::AbsDoubleComplexOfMorphisms) | ||
| lower_bound(D::AbsDoubleComplexOfMorphisms) | ||
| ``` |
There was a problem hiding this comment.
Looking at this, @fieker and me wonder what an actual application of these and related methods would be? I didn't see any in this PR (maybe I missed them, pointers appreciated).
There was a problem hiding this comment.
It is used through vertical_range and horizontal_range in the total_complex routine.
|
I did not feel very confident to introduce a whole new pattern along the lines of |
| @doc raw""" | ||
| finalize!(D::DoubleComplexOfMorphisms) | ||
|
|
||
| Compute all non-zero entries `D[i, j]` and maps of connected components of entries | ||
| of `D` in the grid of the double complex, starting from non-zero entries in | ||
| the cache which have already been computed. | ||
|
|
||
| This can be used to write a full double complex to disc by filling the cache first. | ||
| """ |
There was a problem hiding this comment.
@jankoboehm : This one the other. I'm not 100% sure how to explain all that stuff with the connected components, though. Maybe better to not export this and leave it to the internal people who know what they're doing.
There was a problem hiding this comment.
I don't see a reason why we should not give it to the user, since it is clearly formulated and not ambiguous.
There was a problem hiding this comment.
connected components of a complex? This sounds a bit misleading. What about contiguous pieces?
|
I think I have now talked to all people relevant to this PR except @wdecker . So far, everyone seems to be OK with these changes in general and it is my impression, that this is ready for However, we did not yet talk about the |
|
I removed the morphisms of complexes and added more documentation. If anyone still has any objections, please let me know. Otherwise this should be good to go and I would run the tests for merge. |
|
Think we can go forward with this, if there are not objections, @wdecker ? |
HechtiDerLachs
force-pushed
the
double_complexes
branch
from
9b814c2
to
fc9aebe
Compare
HechtiDerLachs
force-pushed
the
double_complexes
branch
from
fc9aebe
to
9713a1a
Compare
| For double complexes we have some generic functionality available, e.g. | ||
| `total_complex(D::AbsDoubleComplexOfMorphisms{ChainType, MorphismType})`. | ||
| This generic functionality assumes certain methods to be implemented for the | ||
| `ChainType` and the `MorphismType` of the double complex `D`. For instance, | ||
| it must be possible to compose two morphisms of type `<:MorphismType` and | ||
| get a new object of type `<:MorphismType`. Sometimes, the required functionality | ||
| is not streamlined throughout OSCAR (and there is little hope to achieve this). | ||
| One example for this are direct sums: For finitely generated modules, the | ||
| function takes a special keyword argument `task` to indicate whether the inclusion | ||
| and projection maps should also be returned, while for `TorQuadModule`s, this keyword argument | ||
| is not even available. To potentially accomodate all these different types in our | ||
| double complexes, the generic code uses an internal method |
There was a problem hiding this comment.
This paragraph is rather hard to follow, because it mixes general remarks and the specific example of total_complex. It might be better for the reader, if you visibly split general remarks and specific example.
There was a problem hiding this comment.
At this moment, I don't see how, really. Sorry.
There was a problem hiding this comment.
Actually, your small change already made it a lot less confusing.
| Double complexes can be bounded or unbounded. But even in the bounded case where only finitely | ||
| many ``D_{i, j}`` can be non-zero, it is not clear that a concrete bound for the | ||
| range of indices ``(i, j)`` of possibly non-zero entries is known. Then again, even in that case | ||
| such a bound might be rather theoretical and it is still encouraged that double complexes be implemented | ||
| lazy and not fill out the full grid of morphisms on construction. Thus a merely theoretical bound | ||
| on the range of indices does not seem to be practically relevant. |
There was a problem hiding this comment.
If the bounds are not of practical relevance, then why do you spend a whole paragraph on this. Maybe it would be better to state that the double complexes are a priori considered as unbounded, unless a bound is given, inherited or explicitly computed. It should then also be mentioned that lazy evaluation is crucial for handling the bounded and unbounded setting in this unified way.
There was a problem hiding this comment.
The bounds do have practical relevance. Say you have a double complex and you want to compute it's total complex. For every strand (direct sum of the D_{i, j} with i + j = k) there would be infinitely many pairs (i, j) that one would have to check to be zero. So bounding the range in which to look, is key here.
The point here was to say that a theoretical bound is not relevant here. Only concretely known bounds are supported here, i.e. has_upper_bound is true only when a concrete bound is known (and not something like "We know a Groebner basis is finite, but it might also be a million elements").
I wanted to have all this discussion here so that people do not come forward saying: "Have you thought of this? And that?".
There was a problem hiding this comment.
I overhauled this part now.
| @doc raw""" | ||
| finalize!(D::DoubleComplexOfMorphisms) | ||
|
|
||
| Compute all non-zero entries `D[i, j]` and maps of connected components of entries | ||
| of `D` in the grid of the double complex, starting from non-zero entries in | ||
| the cache which have already been computed. | ||
|
|
||
| This can be used to write a full double complex to disc by filling the cache first. | ||
| """ |
There was a problem hiding this comment.
connected components of a complex? This sounds a bit misleading. What about contiguous pieces?
| elseif horizontal_direction(D) == :cochain | ||
| return left_bound(D):right_bound(D) | ||
| end | ||
| error("typ not recognized") |
There was a problem hiding this comment.
typo? 'type'
(and further occurrences)
There was a problem hiding this comment.
seeing the discussion with @fingolfin above, you might have wanted direction instead of type
There was a problem hiding this comment.
typ was what I found in the ComplexOfMorphisms. Not my choice. But I wanted to stay close to the complexes also with terminology. No strong feelings about changing this from my side.
| In order for the generic implementation to work for your specific `ChainType` the following | ||
| needs to be implemented. | ||
|
|
||
| * `morphism_type(ChainType)` must produce the type of morphisms between objects of type `ChainType`; | ||
| * the call signature for `function (fac::TensorProductFactory{ChainType})(dc::AbsDoubleComplexOfMorphisms, i::Int, j::Int)` needs to be overwritten for your specific instance of `ChainType` to produce the `(i, j)`-th entry of the double complex, i.e. the tensor product of `C[i]` and `D[j]`; | ||
| * the call signature for `function (fac::HorizontalTensorMapFactory{ChainType})(dc::AbsDoubleComplexOfMorphisms, i::Int, j::Int)` needs to be overwritten to produce the map on tensor products `C[i] ⊗ D[j] → C[i±1] ⊗ D[j]` induced by the (co-)boundary map on `C` (the sign depending on the `typ` of `C`); | ||
| * similarly for the `VerticalTensorMapFactory`. |
There was a problem hiding this comment.
This sounds like a manual entry targeting programmers. Do you really want it in the docstring of tensor_product?
There was a problem hiding this comment.
I wanted to have it here for as long as we're in the phase of people trying to use this stuff for the first time for new types. It's basically implemented for nothing but modules at this point. So for any new user (like those on slack) I wanted to put this out very prominently to give a hint to where to start implementing things for their own types.
|
Changes done. Have a look whether you approve. If yes, I can run the tests again. |
HechtiDerLachs
force-pushed
the
double_complexes
branch
from
6e960a5
to
936979c
Compare
|
Can this be merged, then? |
After this morning's discussion a first draft of how I think double complexes and their total complexes can effectively be implemented.