Periodic Khalimsky space and pre-Khalimsky space.

[rolanddenis]

Introduction

This PR adds the possibility to specify a different closure type for each dimension, and to define periodic dimensions. In addition, some bugs in KhalimskySpaceND have been fixed.

To ensure that the cells coordinates keep valid (ie inside the boundaries, even for periodic dimensions), many checks have been added in KhalimskySpaceND methods to enforce valid manipulation of the cells.

Therefore, in order to keep the ability of low-level coordinate modification, the new class KhalimskyPreSpaceND model an unbounded cellular grid space that manipulates so-called pre-cell. All his methods are static and allow fast and unchecked read/write of the pre-cells.

Pre-Space

Even if the conversion cell to pre-cell is allowed, the reverse conversion is only possible through dedicated methods of KhalimskySpaceND.

CPreCellularGridSpace is a new concept for KhalimskyPreSpaceND and CCellularGridSpace now depends on CPreCellularGridSpace plus few methods related to the bounds.

Periodic Khalimsky space

Along a periodic dimension, it is always possible to translate a cell without going out of space ((u|s)IsInside returns always true and IsMin & IsMax return always false). The Khalimsky coordinates returned by (u|s)KCoords are always between 2*myLower[k] (like a closed dimension) and 2*myUpper[k]+1 (like an open dimension) where myLower and myUpper are the digital bounds given in the init method. (u|s)GetMin, (u|s)GetMax, (u|s)First and (u|s)Last return cells based on the reference domain bounds (like (u|s)KCoords).

Thus, storage classes based on a periodic Khalimsky space will only see cells in the reference domain (unless for cells whose coordinates were modified by hand, without using KhalimskySpaceND methods). In particular, the cubical complex PR #1079 is compatible with periodic Khalimsky space, at least for the collapsing function.

The previous interface is kept unchanged (in particular the init method with only a bool to specify closed space) and thus, current code using KhalimskySpaceND should work like before.

When using different closure type per dimension, or for periodic spaces, the method isSpaceClosed returns true if every dimension is closed (non-periodic) or periodic. Thus, if isSpaceClosed returns false, it doesn't mean that the space is open.

Checklist

• Documenting KhalimskySpaceND, especially the behaviour of each method with periodic dimension.
• Modifying CCellularGridSpaceND ?
• Adding a template parameter to optimize the code when periodicity is not needed (the current performance impact must be tested).
• Less coordinates verification for periodic dimension in some methods ?
• Checking compatibility of DGtal with periodic Khalimsky spaces (syntax checking):
  • SurfelNeighborhood (comparison with KhalimskySpaceND::min & KhalimskySpaceND::max),
  • SurfelAdjacency,
  • ExplicitDigitalSurface,
  • Surfaces:
    • extractAllPointContours4C (direct coordinates manipulation), ( as an issue )
    • sWriteBoundary (direct coordinates manipulation),
    • uFillInterior (comparison with uGetMax),
    • others.
• 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)
• Testing DGtalTools tools containing KSpace (from grep -rl 'KSpace' *)
• Testing DGtal examples that contains KhalimskySpace or KSpace and comparing it to the master.
• When possible, make classes use KhalimskyPreSpace as default Khalimsky space and/or weaken concept check (to CPreCellularGridSpace). ( as an issue )

Disclaimer

Adding periodic dimensions implies to guarantee validity of the cells (to avoid too high extra computational cost of KhalimskySpaceNDmethods). Therefore, it was needed to deeply refactor this class and that explains this huge PR...
The positive part is that highlights suspicious usages of KhalimskySpaceND (actually, some bugs was discovered) and features strong foundations for future classes that rely on bounded spaces.

[JacquesOlivierLachaud]

It would be nice also to document the user guide to KhalimskySpaceND accordingly (to add to todo list).

[rolanddenis]

I have checked some classes to see if the syntax is compatible with periodic Khalimsky Space. Not compatible syntax is e.g.:

• modifying "by hand" cells coordinate without using afterwards the KhakimskySpaceND methods that guarantee the correctness of the coordinates (like (u|s)Cell, (u|s)KCoords, etc).
• comparing cells coordinates with the result of KhalimskySpaceND::(min|max), (u|s)Get(Min|Max), (u|s)(First|Last) instead of using dedicated methods (u|s)Is(Min|Max).

The checked files and methods are listed in the todo list. Some methods of Surfaces probably need to be fixed.

[dcoeurjo]

@JacquesOlivierLachaud could you please review this PR ?

[JacquesOlivierLachaud]

Looks great to me. Just 3 remarks:

• you should update the dot graph of packageTopologyConcepts1.dox (miss KhalimskyPre...)
• perhaps myCoordinates should be renamed coordinates in preCell ? It is solely if it does not require a lot of changes elsewhere.
• add yourself as author in moduleCellularTopology (author and doc author), so that we can blame you afterwards ;-P

[rolanddenis]

I also see a file named packageTopologyConcepts.dox that seems to be an old mix of packageTopologyConcepts1.dox and packageTopologyConcepts1.dox. Should I remove it ?

[JacquesOlivierLachaud]

Yes, you are right. You can remove it.

[rolanddenis]

Ok. I'll also rename myPositive in positive.

[rolanddenis]

Done

[JacquesOlivierLachaud]

Just fix the conflicts (in packageTopologyConcepts1.dox and KhalimskySpaceND.h) and we can merge.
Have you checked how DGtalTools is impacted ?

[JacquesOlivierLachaud]

At least, all DGtalTools compile with these modifications. I have checked a few tools and they work.

[rolanddenis]

Yes, I have already checked DGtalTools (see the end of this PR's todolist). Actually, this PR helps me to found some bugs in DGtalTools (see DGtal-team/DGtalTools#256) and I have some fixes to pull to DGtalTools now.

[dcoeurjo]

Actually, Travis now tries to compile the DGtaltools for each PR (see Travis.Yml).

[dcoeurjo]

Thanks everyone for this nice PR

[rolanddenis]

Finally, after 6 months 😅