Add LookUpTableFunctions #1155

phcerdan · 2016-03-27T09:44:50Z

Add LookUpTableFunctions

New functions to deal with pre-computed look up tables.
The motivation is to speed up large computation that use predicate
functions depending on voxel neighborhood, such as isSimple.

Add new folder /topology/tables containing:

NeighborhoodConfigurationGenerators, function used to generate the tables.
compressed .tar.gz tables for simplicity.

Modify example generateSimplicityTables3d:

using functions from new header.
create tables for different topologies.
user can choose 26_6,6_26, 18_6 and 6_18 topologies.
Generated tables are heavy, 64MB, so they have been added
to the source code after compressed using:
cmake -E tar cvzf table26_6.tar.gz simple_table26_6.txt resulting in a light (<500KB) tar file.

Add CMake rules:

TablesConfig.cmake dealing with decompress,
copy and install tables. Linked in root CMakeLists.txt
ConfigTables.h.in . CONFIGURE_FILE with strings pointing
to all available tables. The generated ConfigTables.h
change location of tables depending on Build or Install.

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)

JacquesOlivierLachaud · 2016-03-29T10:06:26Z

It seems a good idea to me.
I was wondering if we should not do the same for the 2D case ;-)

phcerdan · 2016-03-29T23:39:47Z

The only concern I have is that every 3D table (with 2^26 configurations) is 64 MB after decompression.
Not sure if it is better to decompress at build/install time (this PR) or include an external library, such as zlib or minizip, to decompress at run time. (there is a boost filter that works as a zlib wrap )

I don't know how keen you are to add the zlib external dependency. For a few tables this approach looks all right though.

@JacquesOlivierLachaud Yeah, 2D tables are straight forward to add, and much lighter. I will push them.

dcoeurjo · 2016-04-10T08:37:14Z

src/DGtal/topology/tables/LookUpTableFunctions.h

+
+namespace DGtal
+{
+ namespace lut {


global functions should be in the DGtal::functions:: namespace.

DGtal::functions::simplicityhas namespace ? lut is too generic IMHO

In a similar way, the filename LookUpTableFunctions is not great.. I have the feeling that it could be confusing. What do you think?

Good point, I will add the functions:: namespace asap.

The motivation of this PR is related to the other PR #1147 , where we use look up tables to calculate the (way more expensive) isIsthmus that depends on the neighborhood of voxels. If lut is too generic, simplicity is not able to capture the isthmus case. Maybe tables::?

I am completely open to suggestions of changing the name, but I cannot really think in anything better or more clarifying/descriptive. The user probably will only interact with this using the loadTable("string_variable_pointing_to_table_file") from ConfigTable.h.in. Other names: NeigborhoodConfigurations, PreComputedTables, PreComputedConfiguration ? Not sure, sorry!!

😄 no idea, too

NeigborhoodConfigurations is ok for the filename but I don't know for the namespace... neighborhood:: ?

Why not a name related to the objective of lut... here topology, homotopy, simplicity, etc ... as you wish

----- Mail original -----

De: "David Coeurjolly" [email protected]
À: "DGtal-team/DGtal" [email protected]
Cc: "Jacques-Olivier Lachaud" [email protected]
Envoyé: Lundi 11 Avril 2016 11:16:01
Objet: Re: [DGtal-team/DGtal] Add LookUpTableFunctions (#1155)

In src/DGtal/topology/tables/LookUpTableFunctions.h :

+#if !defined LookUpTableFunctions_h
+/** Prevents repeated inclusion of headers. */
+#define LookUpTableFunctions_h
+//////////////////////////////////////////////////////////////////////////////
+// Inclusions
+#include
+#include
+#include
+#include "DGtal/topology/Object.h"
+#include "DGtal/helpers/StdDefs.h"
+#include <unordered_map>
+#include "boost/dynamic_bitset.hpp"
+
+namespace DGtal
+{

namespace lut {

no idea, too

NeigborhoodConfigurations is ok for the filename but I don't know for the namespace... neighborhood:: ?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

dcoeurjo · 2016-04-11T09:20:08Z

could you please also consider to add something in the doc (moduleDigitalTopology.dox, last sections) ?
thx

dcoeurjo · 2016-04-11T09:21:07Z

BTW, maybe the dgtaltools should also be updated (ping @kerautret )

phcerdan · 2016-04-12T00:20:28Z

What do you prefer NeigborhoodConfigurations or NeighborhoodPredicates for the file name?
I am moving NeigborhoodXXX.h/ih into the folder topology/ and put the compressed tables and the configure file NeighborhoodTables.h.in in topology/tables ... better?

I like the split of the namespaces for the different tables.
simplicity sounds great, but for the Isthmus case? topology ? Or isthmuscity :)?

I will update the docs, and the log meanwhile.

phcerdan · 2016-04-18T00:46:21Z

Last two commits:
Rename files.

Add object interface to tables with setTable method: it sets CowPtr<T> members with a Clone argument.
Modified isSimple to use the tables if setTable has been called.
No new constructors added to Object.
Add docs with an extract of testObject.cpp using the setTable interface.
Split header into helpers such as getOccupancyConfiguration for object and complex (to avoid recursive inclusion). And also the generateTables is there.
Changed loadTable from std::shared_ptr<dynamic_bitset> to CountedPtr<dynamic_bitset> to fit better with the Alias<dynamic_bitset> argument of setTable.

Rebased all branch commits

New functions to deal with pre-computed look up tables. Motivation is to speed up large computation that use predicate functions depending on voxel neighborhood, such as isSimple. Add new folder /topology/tables containing: - NeighborhoodConfigurations.h loading table and map. - NeighborhoodConfigurationsHelpers.h helpers for Object and CubicalComplex. - compressed .tar.gz tables for simplicity. Modify example generateSimplicityTables3D, and 2D: - using functions from new header. - create tables for different topologies. - user can choose 26_6,6_26, 18_6 and 6_18 topologies in 3D and 8_4,4_8 in 2D. - Generated tables are then compressed using: `cmake -E tar cvzf table26_6.tar.gz simple_table26_6.txt` Add CMake rules: - NeighborhoodTablesConfig.cmake dealing with decompress, copy and install tables. Linked in root CMakeLists.txt - NeighborhoodTablesConfig.h.in . CONFIGURE_FILE with strings pointing to all available tables. The generated NeighborhoodTablesConfig.h change location of tables depending on Build or Install.

JacquesOlivierLachaud · 2016-04-25T15:14:54Z

cmake/NeighborhoodTablesConfig.cmake

+foreach(zip_table ${DGTAL_TABLES_COMPRESSED})
+ execute_process(
+ COMMAND ${CMAKE_COMMAND} -E tar xzf ${zip_table}
+ WORKING_DIRECTORY ${unzip_folder}


Does this work on Windows ? Should we add something in the Windows install guide (like install tar archive tool) ?

CMake provides it, and it is mulit-platform, but I am not really sure what version of cmake is required.

JacquesOlivierLachaud · 2016-04-26T12:12:06Z

Nice PR. Sorry for the long delay for reviewing it, I had several deadlines in the last weeks (SGP and ICPR). I check that everything compiles on my computer and the documentation and I merge.

phcerdan · 2016-04-26T12:19:03Z

Thanks a lot for the reviewing and feedback, I am learning a lot with this library! I am right now contributing some segmentation filter in ITK, that I need for my thesis, but will try to improve this and the PR #1147 as soon as possible.

JacquesOlivierLachaud · 2016-04-26T12:20:21Z

It is very nice for us to have contributors from far (far) away ! I was really surprised to see somebody from New Zealand who knows about isthmus and such stuff (so french :) )

JacquesOlivierLachaud · 2016-04-26T12:21:06Z

Do you know Reinhardt Klette (he is in Auckland) ? He is one of the few guy in our community. Well, he is retiring at the moment I think.

phcerdan · 2016-04-26T12:37:12Z

No, I don't know him, I have the bad excuse that I am closer to Wellington. And yeah I know it is far away hehe, I am Spanish! Currently doing a PhD here in the antipodes, in a biophysics group , but I have been migrating to computer vision to characterize geometry/connectivity/topology of biopolymer networks

JacquesOlivierLachaud · 2016-04-26T12:45:27Z

Everything is fine for me and it works nicely on my computer. I wait for the few minor fixes (change of names mostly) and I merge.
And I think it will be time for you to go to bed, no ? :-)

phcerdan · 2016-04-26T12:56:40Z

haha indeed. I will make the changes soon. Have a good afternoon!

* Add typedef NeighborhoodConfiguration = uint_32 in topology/helpers/NeighborhoodConfigurationHelper.h. * Remove NeighborhoodConfigurationHelpers.h. * Move generateTable to topology/tables/NeighborhoodConfigurationGenerators. * Rename and move getOccupancyXXX functions, to Object class function and CubicalComplexFunctions respectively. * Fix indendation in testObject. * Rename and add documentation to helper function mapping zero neighborhood points to configuration bit masks.

JacquesOlivierLachaud · 2016-05-03T12:21:56Z

src/DGtal/topology/CubicalComplexFunctions.h

+ /**
+ * Get the occupancy configuration of the neighborhood of a point in a cubical
+ * complex.
+ * The neighborhood size is considered DxD for dimension D of the point


You mean 3^d in dD, so 3x3 in 2D and 3x3x3 in 3D.

thx, changed.

JacquesOlivierLachaud · 2016-05-03T12:25:48Z

Except from the above remark, it is fine for me to merge.

JacquesOlivierLachaud · 2016-05-04T07:02:28Z

Fine. I merge.

phcerdan force-pushed the look-up-tables branch from 815cd37 to 4c98094 Compare March 27, 2016 11:35

dcoeurjo self-assigned this Mar 28, 2016

dcoeurjo added ProjectManagment Topology labels Mar 28, 2016

phcerdan force-pushed the look-up-tables branch from 4300992 to d792693 Compare March 30, 2016 07:35

phcerdan mentioned this pull request Apr 4, 2016

VoxelComplex: CriticalKernels framework #1147

Merged

12 tasks

dcoeurjo reviewed Apr 10, 2016
View reviewed changes

phcerdan force-pushed the look-up-tables branch 3 times, most recently from 2e2452d to 601e5d4 Compare April 17, 2016 13:40

phcerdan force-pushed the look-up-tables branch from 3e72d5e to f2f500d Compare April 19, 2016 05:44

JacquesOlivierLachaud reviewed Apr 25, 2016
View reviewed changes

phcerdan added 2 commits April 28, 2016 18:59

Fix shadow error

3aa874a

JacquesOlivierLachaud reviewed May 3, 2016
View reviewed changes

Fix docs of getSpelNeighborhoodConfiguration.

acb7484

JacquesOlivierLachaud merged commit 1211956 into DGtal-team:master May 4, 2016

phcerdan deleted the look-up-tables branch January 28, 2017 03:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LookUpTableFunctions #1155

Add LookUpTableFunctions #1155

phcerdan commented Mar 27, 2016 •

edited

Loading

JacquesOlivierLachaud commented Mar 29, 2016

phcerdan commented Mar 29, 2016

dcoeurjo Apr 10, 2016

dcoeurjo Apr 10, 2016

dcoeurjo Apr 10, 2016

phcerdan Apr 10, 2016

dcoeurjo Apr 11, 2016

JacquesOlivierLachaud Apr 11, 2016

dcoeurjo commented Apr 11, 2016

dcoeurjo commented Apr 11, 2016

phcerdan commented Apr 12, 2016

phcerdan commented Apr 18, 2016 •

edited

Loading

JacquesOlivierLachaud Apr 25, 2016

phcerdan Apr 25, 2016

JacquesOlivierLachaud commented Apr 26, 2016

phcerdan commented Apr 26, 2016

JacquesOlivierLachaud commented Apr 26, 2016

JacquesOlivierLachaud commented Apr 26, 2016 •

edited

Loading

phcerdan commented Apr 26, 2016

JacquesOlivierLachaud commented Apr 26, 2016

phcerdan commented Apr 26, 2016

JacquesOlivierLachaud May 3, 2016

phcerdan May 4, 2016

JacquesOlivierLachaud commented May 3, 2016

JacquesOlivierLachaud commented May 4, 2016

Add LookUpTableFunctions #1155

Add LookUpTableFunctions #1155

Conversation

phcerdan commented Mar 27, 2016 • edited Loading

Add LookUpTableFunctions

Add new folder /topology/tables containing:

Modify example generateSimplicityTables3d:

Add CMake rules:

Checklist

JacquesOlivierLachaud commented Mar 29, 2016

phcerdan commented Mar 29, 2016

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

dcoeurjo commented Apr 11, 2016

dcoeurjo commented Apr 11, 2016

phcerdan commented Apr 12, 2016

phcerdan commented Apr 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JacquesOlivierLachaud commented Apr 26, 2016

phcerdan commented Apr 26, 2016

JacquesOlivierLachaud commented Apr 26, 2016

JacquesOlivierLachaud commented Apr 26, 2016 • edited Loading

phcerdan commented Apr 26, 2016

JacquesOlivierLachaud commented Apr 26, 2016

phcerdan commented Apr 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JacquesOlivierLachaud commented May 3, 2016

JacquesOlivierLachaud commented May 4, 2016

phcerdan commented Mar 27, 2016 •

edited

Loading

phcerdan commented Apr 18, 2016 •

edited

Loading

JacquesOlivierLachaud commented Apr 26, 2016 •

edited

Loading