CAVEconnectome / MeshParty

Apache License 2.0

35 stars 16 forks source link

Fix mesh csgraph distances #30

Closed ceesem closed 5 years ago

ceesem commented 5 years ago

mesh.csgraph had weights that were typically twice that of the distance between vertices, due to double counting the bidirectional edges between vertices on a face. This PR fixes that by turning off the double counting (i.e. setting directed=True in create_csgraph) and making link_edges similarly bidirectional when they count toward graph_edges. To avoid double counting, I also check that make sure that the naive bidirectional link_edges doesn't have duplicate edges when used to make the csgraph.

ceesem commented 5 years ago

I'll look into the test results shortly...

fcollman commented 5 years ago

Looks like it is just a change in Skeleton behavior... not clear which is better. It’s an interesting difference favoring short mesh hops

codecov-io commented 5 years ago

Codecov Report

Merging #30 into master will increase coverage by 0.05%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
+ Coverage   75.88%   75.93%   +0.05%     
==========================================
  Files          11       11              
  Lines        1704     1708       +4     
==========================================
+ Hits         1293     1297       +4     
  Misses        411      411

Impacted Files	Coverage Δ
meshparty/trimesh_io.py	`74.58% <100%> (+0.24%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8579daa...70ed109. Read the comment docs.

ceesem commented 5 years ago

A bit unclear about what changed since tests were written, but the existing branch points look totally right on inspection of the skeletonization and the current full_cell_with_ais.png output is slightly rotated relative to the file that was in there before. Tests pass now and the results look good to me.

ceesem commented 5 years ago

I'm going to close this as a PR into master and re-open it as a PR into fcc_skel_quality, since the scope has changed a bit due to usage needs.