Contiguity for morphisms of simplicial complexes

sagemath / sage

Main repository of SageMath

https://www.sagemath.org

Other

1.38k stars 470 forks source link

Contiguity for morphisms of simplicial complexes #21847

Closed jhpalmieri closed 7 years ago

jhpalmieri commented 7 years ago

Implement a check for whether two morphisms of simplicial complexes are contiguous.

CC: @tscrim @fchapoton

Component: algebraic topology

Author: John Palmieri

Branch/Commit: 4c43aa4

Reviewer: Travis Scrimshaw

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

jhpalmieri commented 7 years ago

Branch: u/jhpalmieri/contiguity

jhpalmieri commented 7 years ago

New commits:

99a1cb6 Check whether two morphisms of simplicial complexes are contiguous.

jhpalmieri commented 7 years ago

Commit: 99a1cb6

tscrim commented 7 years ago

comment:4

I've made some reviewer changes, but you are currently inconsistent. If the user inputs something that is not a map, we raise a ValueError, but if the map has differing domain and codomains, then you return False. In both cases, the input is nonsensical, and I would argue (just like for is_similar for matrices) that the return should be False in both cases because they are not contiguous. Thoughts?

New commits:

415cb0d Some reviewer changes.

tscrim commented 7 years ago

Changed commit from 99a1cb6 to 415cb0d

tscrim commented 7 years ago

Reviewer: Travis Scrimshaw

tscrim commented 7 years ago

Changed branch from u/jhpalmieri/contiguity to u/tscrim/contiguity-21847

jhpalmieri commented 7 years ago

Changed branch from u/tscrim/contiguity-21847 to u/jhpalmieri/contiguity-21847

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2ad7a2d trac 21847: if the two maps don't have the same domain and codomain,

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 415cb0d to 2ad7a2d

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

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

4c43aa4 trac 21847: fix TESTS

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 2ad7a2d to 4c43aa4

jhpalmieri commented 7 years ago

comment:8

I don't have strong feelings about it, so I don't mind returning False in both cases. The original version was intentionally inconsistent: it may make sense to ask if any two simplicial maps are contiguous, but it doesn't make sense to ask if a map is contiguous to a matrix.

jhpalmieri commented 7 years ago

comment:9

By the way, is_similar raises an error if you plug in a non-matrix, but it just returns False if you plug in a matrix of the wrong shape. So the original behavior was more like is_similar than the new version.

fchapoton commented 7 years ago

comment:10

Travis was referring to #18505 (needs review), and to the controversy and vote on sage-devel about what to do with bad input there.

jhpalmieri commented 7 years ago

comment:11

Right, I remember that now. The issue with is_similar was how to handle matrices of different shapes, which I think is more like how to handle morphisms of simplicial complexes with mismatched domains and/or codomains (return False, I say), rather than how to handle inputs which aren't even morphisms (raise an error in my original version here, not that I care too much).

tscrim commented 7 years ago

comment:12

LGTM. Thanks.

vbraun commented 7 years ago

Changed branch from u/jhpalmieri/contiguity-21847 to 4c43aa4