Par dir collapse #1199

copyme · 2016-07-25T18:36:39Z

Thanks a lot for contributing to DGtal, before submitting your PR, please fill up the description and make sure that all checkboxes are checked. Please remove these lines in your PR.

PR Description

Same as #1111

Checklist

Unit-test of your feature with Catch.
Doxygen documentation of the code completed (classes, methods, types, members...)
Documentation module page added or updated.
New entry in the ChangeLog.md added.
No warning raised in Debug cmake mode (otherwise, Travis C.I. will fail).
All continuous integration tests pass (Travis & appveyor)

…DirCollapse

copyme · 2016-09-28T09:28:22Z

OK, I think everything is here.

copyme · 2016-12-02T21:01:26Z

Hey guys do you plan to add this to 0.9.3?

dcoeurjo · 2016-12-03T07:34:14Z

We should :) sorry for being late for the PR review.

dcoeurjo

thanks again for the PR.
@JacquesOlivierLachaud can you please have a look to it too ? thx

dcoeurjo · 2016-12-21T19:09:15Z

ChangeLog.md

@@ -86,6 +86,9 @@
 - *Topology Package*
 - Add pre-calculated look up tables to speed up Object::isSimple calculations.
   (Pablo Hernandez-Cerdan, [#1155](https://github.com/DGtal-team/DGtal/pull/1155))
+ -  Implementation of ParDirCollapse with CollapseSurface and CollapseIsthmus.


Could you please move this to the 0.9.3 release section ?

dcoeurjo · 2016-12-21T19:10:27Z

src/DGtal/doc/global.bib

@@ -910,3 +910,15 @@ @article{kerautret_meaningful_2012
 	pages = {2379--2392}
 }

+
+@inproceedings{Chaussard:IWCIA:09,


Is there a "journal" version of this publication ?

Up to my knowledge there is no journal version.

dcoeurjo · 2016-12-21T19:11:32Z

src/DGtal/topology/ParDirCollapse.h

+/**
+ * Description of class 'ParDirCollapse' <p>
+ * \brief Aim: Implements thinning algorithm in cubical complexes.
+ * Paper: Chaussard, J. and Couprie, M., Surface Thinning in 3D Cubical Complexes,


could you please add a short description (well, a bit longer than just the ref 😃) ?

dcoeurjo · 2016-12-21T19:12:19Z

src/DGtal/topology/ParDirCollapse.h

+    /// Any model of concepts::CCellularGridSpaceND, i.e. a type that models a Khalimsky space.
+    typedef typename CC::KSpace KSpace;
+    /// Type of Nd integer vector
+    typedef PointVector< KSpace::dimension, int > Vector;


I think this type can be derived from inner types of CubicalComplex

no need to define from PointVector

dcoeurjo · 2016-12-21T19:13:15Z

src/DGtal/topology/ParDirCollapse.h

+    ParDirCollapse ( const KSpace & k );
+
+     /**
+     * @param pComplex -- Cubical complex


add description.

dcoeurjo · 2016-12-21T19:13:44Z

src/DGtal/topology/ParDirCollapse.h

+    unsigned int eval ( unsigned int iterations );
+
+    /**
+     * Extension of basic algorithm which preserve KSpace::dimension - 1 faces which are not included


++description

The basic algorithm is named right now.

dcoeurjo · 2016-12-21T19:14:26Z

src/DGtal/topology/ParDirCollapse.h

+     * @param F -- cell of a dimension one lower than G.
+     * @param G -- cell of a dimension one higher than F.
+     */
+    int getOrientation ( const Cell& F, const Cell& G );


shouldn't this method be const ?

can you please check others?

Sure. Fixed for another methods too.

dcoeurjo · 2016-12-21T19:16:01Z

src/DGtal/topology/doc/moduleCubicalComplex.dox

+@section dgtal_ccomplex_sec8 Thinning in cubical complexes
+
+Thinning of digital objects in 2D and 3D with the guarantee that a final skeleton is thin can be achieved directly
+only in \f$\mathbb{Z}^2\f$ for so-called well-composed images. Nevertheless, the problem can be solved for 3D objects


ref to define well-composed

dcoeurjo · 2016-12-21T19:16:31Z

src/DGtal/topology/doc/moduleCubicalComplex.dox

+
+The first scheme, so-called ParDirCollapse---parallel directional collapse---based on the idea such that free pairs
+of faces are collapsed with respect to a fixed order of directions and order of face dimensions. In other words, in
+each iteration we first collapse free paris of a given direction starting with pairs of the highest dimension.


paris -> pairs

dcoeurjo · 2016-12-21T19:17:29Z

tests/topology/testParDirCollapse.cpp

+    {
+      getComplex< CC, KSpace > ( complex, K );
+      thinning.attach ( &complex );
+      thinning.collapseSurface ();


well.. this is not really a unit test without any REQUIRE ;)

You may check for instance the euler number before and after collapse(s) operations.

Done. Thanks for the suggestion.

JacquesOlivierLachaud

I have checked the files, but not their execution (I will see tomorrow). There are minor comments to fix before merging.

JacquesOlivierLachaud · 2016-12-21T21:32:05Z

tests/topology/testParDirCollapse.cpp

+    {
+      getComplex< CC, KSpace > ( complex, K );
+      thinning.attach ( &complex );
+      thinning.collapseSurface ();


You may check for instance the euler number before and after collapse(s) operations.

JacquesOlivierLachaud · 2016-12-21T21:32:25Z

tests/topology/testParDirCollapse.cpp

+      getComplex< CC, KSpace > ( complex, K );
+      thinning.attach ( &complex );
+      thinning.collapseIsthmus ();
+    }


Done. Thanks for the suggestion.

JacquesOlivierLachaud · 2016-12-21T21:33:06Z

src/DGtal/topology/ParDirCollapse.ih

+DGtal::ParDirCollapse< CC >::isIsthmus ( CellMapConstIterator F )
+{
+    Cells faces = K.uLowerIncident ( F->first );
+    for ( unsigned int i = 0; i < faces.size(); i++ )


I think there is a type Size in CubicalComplex (instead of unsigned int).

JacquesOlivierLachaud · 2016-12-21T21:33:21Z

src/DGtal/topology/ParDirCollapse.ih

+DGtal::ParDirCollapse< CC >::isNotIncludedInUpperDim ( CellMapConstIterator F )
+{
+    Cells faces = K.uUpperIncident ( F->first );
+    unsigned int dim = K.uDim ( F->first ) + 1;


Type Dimension instead ?

JacquesOlivierLachaud · 2016-12-21T21:34:22Z

src/DGtal/topology/ParDirCollapse.ih

+DGtal::ParDirCollapse< CC >::getOrientation( const Cell& F, const Cell& G )
+{
+    Vector V = K.uKCoords ( F ) - K.uKCoords ( G );
+    for ( unsigned int i = 0; i < K.dimension; i++ )


Type Dimension instead of unsigned int.

JacquesOlivierLachaud · 2016-12-21T21:40:54Z

src/DGtal/topology/doc/moduleCubicalComplex.dox

+number of iteration is not reach. Note in the case of ParDirCollapse thinness of the result depends on the number
+of iterations.
+
+The below steps present how to use ParDirCollapse in DGtal:


The steps below ...

JacquesOlivierLachaud · 2016-12-21T21:42:18Z

src/DGtal/topology/doc/moduleCubicalComplex.dox

+The first scheme, so-called ParDirCollapse---parallel directional collapse---based on the idea such that free pairs
+of faces are collapsed with respect to a fixed order of directions and order of face dimensions. In other words, in
+each iteration we first collapse free paris of a given direction starting with pairs of the highest dimension.
+When there is no more free pairs to remove for a given direction then another direction is considered until specified


until the specified number of iterations is reached ?
or
unless the specified number of iterations is not reached ?

JacquesOlivierLachaud · 2016-12-21T21:42:27Z

src/DGtal/topology/doc/moduleCubicalComplex.dox

+of faces are collapsed with respect to a fixed order of directions and order of face dimensions. In other words, in
+each iteration we first collapse free paris of a given direction starting with pairs of the highest dimension.
+When there is no more free pairs to remove for a given direction then another direction is considered until specified
+number of iteration is not reach. Note in the case of ParDirCollapse thinness of the result depends on the number


the thinness ...

JacquesOlivierLachaud · 2016-12-21T21:42:56Z

src/DGtal/topology/doc/moduleCubicalComplex.dox

+
+The first scheme, so-called ParDirCollapse---parallel directional collapse---based on the idea such that free pairs
+of faces are collapsed with respect to a fixed order of directions and order of face dimensions. In other words, in
+each iteration we first collapse free paris of a given direction starting with pairs of the highest dimension.


paris => pairs

JacquesOlivierLachaud · 2016-12-21T21:44:22Z

src/DGtal/topology/doc/moduleCubicalComplex.dox

+iteration of ParDirCollapse, we mark as non-collapsible those faces which are of dimension dim\f$(X) - 1\f$ and not
+included in faces of higher dimension. Moreover, all sub-faces of dimension \f$dim(X) - 2\f$ of those faces
+have to be not free -- are included in other faces of dimension \f$dim(X) - 1\f$. The final output is
+guarantee to be thin i.e. there is no faces of \f$dim(X)\f$.


JacquesOlivierLachaud · 2016-12-23T16:53:40Z

Hello Kacper. Did you commit the last changes ?
I don't see them on github (or I do not use well the new review system).

copyme · 2016-12-23T20:37:28Z

Hey @JacquesOlivierLachaud not yet. I have some problems here with building and have not had time to check this. I will try to finish tonight.

copyme · 2016-12-23T22:42:30Z

I pushed but I could not compile on my machine. Some other files gave some strange errors. In meanwhile I've had OS upgrade so I guess this is related. Well, lets see if it will pass tests.

copyme · 2016-12-24T11:28:42Z

OK @JacquesOlivierLachaud it is ready for review.

dcoeurjo

Fine for me.. thanks.
@JacquesOlivierLachaud, is it fine for you?

JacquesOlivierLachaud · 2017-01-19T15:40:40Z

Seems perfect to me. I just suggest to add a link to this example in the documentation of the module CubicalComplex, (file topology/doc/moduleCubicalComplex.doc), for instance in the line:

The following programs are related to this documentation: cubical-complex-collapse.cpp, cubical-complex-illustrations.cpp, testCubicalComplex.cpp, here.

Added related file.

copyme · 2017-01-19T15:50:52Z

@JacquesOlivierLachaud @dcoeurjo Done.

missing space

JacquesOlivierLachaud · 2017-01-19T15:54:51Z

Wait for travis/appveyor and I merge. Good work !

dcoeurjo · 2017-01-19T16:01:59Z

+1

copyme · 2017-01-19T19:09:32Z

One test failed because my repository has not been synched with master for a while. Should I merge with master?

dcoeurjo · 2017-01-20T07:08:31Z

No should be fine anyway. Merging.
thanks a lot !

Kacper Pluta and others added 29 commits January 14, 2016 15:02

Initial version

74470f7

First version which compiles after clean ups.

cee00f0

Some clean ups

91e82c8

Refactorization. Small bug fix.

617e2ad

Refactorization.

8b05c3e

Assertions, types, cleanups

ef1d3dd

small fix for priority

ec7b695

Fixes for bugs introduced in the previous commits.

49a5d3b

check for a concept

d62b494

fix for "control may reach end of non-void function"

759e03e

example which shows how to use ParDirCollapse

284b1b9

removed useless include

d161cb4

same as before

382707b

test for ParDirCollapse and example enabled in CMake

c2258c3

formatting

51db3e9

Merge branch 'master' of https://github.com/DGtal-team/DGtal into Par…

3dc03e9

…DirCollapse

Merge branch 'master' of https://github.com/DGtal-team/DGtal into Par…

e8ac525

…DirCollapse

fixes for types shadowing.

271d643

Merge branch 'master' of https://github.com/DGtal-team/DGtal into Par…

d55aacc

…DirCollapse

fix for unused variables

4652482

missing authors

cc6912a

merge

c559d83

Documentation for ParDirCollapse

3c6fb6e

sync of tags for doc

d295475

fixes for captions of images

b504056

revert and add new entry

92c3c7d

Merge branch 'master' of https://github.com/DGtal-team/DGtal

a4632f2

Merge branch 'master' into ParDirCollapse

fdb8b7c

Entry in the changelog and fix for a typo

ed4f588

dcoeurjo requested a review from JacquesOlivierLachaud December 14, 2016 07:37

dcoeurjo added the Topology label Dec 14, 2016

dcoeurjo self-requested a review December 21, 2016 19:08

dcoeurjo reviewed Dec 21, 2016

View reviewed changes

JacquesOlivierLachaud reviewed Dec 21, 2016

View reviewed changes

copyme force-pushed the ParDirCollapse branch from d9c346e to ed4f588 Compare December 23, 2016 12:05

copyme added 2 commits December 23, 2016 23:38

typos and missing references

f6f672c

Descriptions, fixes for types and few others minor changes.

6feb84a

copyme added 2 commits December 24, 2016 11:05

Type are now correctly resolved.

f5a1b17

tautology fixed.

0aa2f66

dcoeurjo approved these changes Jan 19, 2017

View reviewed changes

Update moduleCubicalComplex.dox

11bd0bf

Added related file.

Update moduleCubicalComplex.dox

4fe3e78

missing space

dcoeurjo merged commit cdb4073 into DGtal-team:master Jan 20, 2017

Par dir collapse #1199

Par dir collapse #1199

Conversation

copyme commented Jul 25, 2016 • edited

PR Description

Checklist

copyme commented Sep 28, 2016

copyme commented Dec 2, 2016

dcoeurjo commented Dec 3, 2016

dcoeurjo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JacquesOlivierLachaud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JacquesOlivierLachaud commented Dec 23, 2016

copyme commented Dec 23, 2016

copyme commented Dec 23, 2016

copyme commented Dec 24, 2016

dcoeurjo left a comment

Choose a reason for hiding this comment

JacquesOlivierLachaud commented Jan 19, 2017

copyme commented Jan 19, 2017

JacquesOlivierLachaud commented Jan 19, 2017

dcoeurjo commented Jan 19, 2017

copyme commented Jan 19, 2017

dcoeurjo commented Jan 20, 2017

copyme commented Jul 25, 2016 •

edited