Skip to content
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

Chain homotopies #19179

Closed
jhpalmieri opened this issue Sep 10, 2015 · 31 comments
Closed

Chain homotopies #19179

jhpalmieri opened this issue Sep 10, 2015 · 31 comments

Comments

@jhpalmieri
Copy link
Member

This ticket add chain homotopies, chain contractions, and duals of chain maps and chain homotopies. This is a dependency for #6101 and #6102.

CC: @tscrim @fchapoton

Component: algebraic topology

Keywords: homotopy

Author: John Palmieri

Branch/Commit: 4d53526

Reviewer: Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/19179

@jhpalmieri
Copy link
Member Author

Branch: u/jhpalmieri/chains

@jhpalmieri
Copy link
Member Author

New commits:

dece275trac 19179: chain homotopies, chain contractions, and duals of chain maps

@jhpalmieri
Copy link
Member Author

Commit: dece275

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 10, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

d7b9ed4change AssertionError to other errors

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 10, 2015

Changed commit from dece275 to d7b9ed4

@jhpalmieri
Copy link
Member Author

Dependencies: #6102

@jhpalmieri
Copy link
Member Author

comment:4

It's too messy for me to disentangle this from #6102, so that ticket and this are now dependencies of each other. In particular, the most natural way to construct examples of chain contractions is to use the algebraic topological model of a complex (which is part of #6102). So some of the examples here use code that is part of #6102. The actual code here doesn't depend on #6102, just those few examples.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 11, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

d65ba8dtrac 19179: change `_repr_` for chain maps, chain homotopies

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 11, 2015

Changed commit from d7b9ed4 to d65ba8d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 11, 2015

Changed commit from d65ba8d to f70e3a2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 11, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

f70e3a2trac 19179: doctest fixes for `_repr_` changes

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

4e50776trac 19179: hashing of chain maps, chain homotopies

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2015

Changed commit from f70e3a2 to 4e50776

@jhpalmieri
Copy link
Member Author

comment:8

#18246 broke the default hashing of chain homotopies, so I've added a __hash__ method, and also one for chain maps.

@fchapoton
Copy link
Contributor

comment:9

This looks strange, in chain_complex_morphism.py:

+        self._matrix_dictionary = {}
+        for i in matrices:
+            m = matrices[i]
+            # Use immutable matrices because they're hashable.
+            m.set_immutable()
+            self._matrix_dictionary[i] = m
         self._matrix_dictionary = matrices

Should the last line be removed ?

@jhpalmieri
Copy link
Member Author

comment:10

Yes, thank you.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 14, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

07c9c42trac 19179: delete extra line

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 14, 2015

Changed commit from 4e50776 to 07c9c42

@cnassau
Copy link

cnassau commented Sep 23, 2015

comment:12

I've been playing around with this for the last week and I'm essentially ready to give the ticket a positive review. I can think of a couple of very minor improvements that might be better addressed in a follow-up ticket:

  • add syntactic sugar to replace H._g with something like H[1], or H.endpoint(1) or whatever
  • create a parent for chain homotopies so that linear combinations can be taken easily

I encountered one more serious surprise in my experiments, and I'm not quite sure how to address this: I was starting with a chain complex C over the integers, and I wanted to work with its reduction modulo 2. The code that I came up with actually messed up the original complex:

sage: d1 = matrix([[-1,1,0],[1,-1,0],[-1,0,1],[1,0,-1]]).transpose()
sage: d2 = matrix([[-1,-1,+1,+1],[1,1,1,1]]).transpose()
sage: C=ChainComplex({2:d2,1:d1},degree_of_differential=-1,base_ring=ZZ)
sage: ascii_art(C)
                                [-1  1]      
            [-1  1 -1  1]       [-1  1]      
            [ 1 -1  0  0]       [ 1  1]      
            [ 0  0  1 -1]       [ 1  1]      
 0 <-- C_0 <-------------- C_1 <-------- C_2 <-- 0 
sage: D=ChainComplex(C._diff,degree_of_differential=C.degree_of_differential(),base_ring=GF(2))
sage: ascii_art(C)
                            [1 1]      
            [1 1 1 1]       [1 1]      
            [1 1 0 0]       [1 1]      
            [0 0 1 1]       [1 1]      
 0 <-- C_0 <---------- C_1 <------ C_2 <-- 0 

I would have expected an error here, and possibly a more convenient way to carry out the base change.

As for the patchbot errors: these are a bit confusing; I think they're all due to the fact that the dependencies of the ticket have not been properly merged for the test. Given the dependencies with #6101, #6102 I wonder if it isn't the best approach to approve them all at once, eventually.... (?)

@jhpalmieri
Copy link
Member Author

comment:13

With your example, using D = Chain Complex(C.differential(), ...) works better than with C._diff, because C.differential() returns a copy of the defining data, and so modifying it won't change C. It can be dangerous using hidden attributes, and the corresponding methods ought to be safer.

If you are willing to give this a positive review, then #6101 should come next, but neither will get merged without the other.

@fchapoton
Copy link
Contributor

comment:14

a few remarks on the doc:

  • please remove the dot at the end of the title line of the new file.

  • Please remove the \circ in H \circ H at several places where it appears (or, if you really prefer, put such a composition symbol everywhere else), for coherence with other compositions.

  • What happens in chain homotopy if the given g is None ? This should be explained in the doc.

  • The def of "dual" of chain homotopy class should start with r"""

  • In dual for chain contraction, skip one line after the first sentence

@fchapoton
Copy link
Contributor

Changed keywords from none to homotopy

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2015

Changed commit from 07c9c42 to 4d53526

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

4d53526trac 19179: minor doc fixes

@jhpalmieri
Copy link
Member Author

comment:16

Replying to @fchapoton:

a few remarks on the doc:

  • please remove the dot at the end of the title line of the new file.

Done

  • Please remove the \circ in H \circ H at several places where it appears (or, if you really prefer, put such a composition symbol everywhere else), for coherence with other compositions.

Done (removed \circ)

  • What happens in chain homotopy if the given g is None ? This should be explained in the doc.

This was already there, but I added a sentence.

  • The def of "dual" of chain homotopy class should start with r"""

Right, fixed.

  • In dual for chain contraction, skip one line after the first sentence

Done.

@fchapoton
Copy link
Contributor

comment:17

ok, thanks. Then good to go. It will of course be pending until #6102 is reviewed.

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@fchapoton fchapoton removed this from the sage-6.9 milestone Oct 7, 2015
@tscrim tscrim added this to the sage-6.10 milestone Oct 15, 2015
@tscrim tscrim removed the pending label Oct 15, 2015
@vbraun
Copy link
Member

vbraun commented Oct 17, 2015

comment:20

Circular dependency, please make up your mind what you want merged first

@fchapoton
Copy link
Contributor

comment:21

This one first please, but it will not pass the doctests alone.

@fchapoton
Copy link
Contributor

Changed dependencies from #6102 to none

@vbraun
Copy link
Member

vbraun commented Oct 18, 2015

Changed branch from u/jhpalmieri/chains to 4d53526

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants