Adding support for customizing ITK namespace

Plan

We propose to update ITK to support customizing the ITK namespace.

Motivation

These change will allow importing itk python modules into application themselves built against ITK and ensure filter execution return valid results.

While prior outstanding efforts (PR-3156, PR-3330 , PR-3478 and many more) helped to allow importing itk modules into application like Slicer, the root issue is that symbols are bound to the one already available in the ITK libraries loaded by Slicer.

Running the application, specifying LD_DEBUG_OUTPUT=/tmp/reloc-bindings LD_DEBUG=reloc,bindings and importing itk allows to confirm this.

Here is a portion of the output illustrating that the symbol itk::Object::New() is resolved against libITKCommon-5.3.so.1 built by Slicer instead of _ITKCommonPython.so library provided in the wheel.

   2595154:     binding file 
     /home/jcfr/Downloads/itk-wheels-installed/lib/python3.9/site-packages/itk/_ITKCommonPython.so [0] 
   to 
     /home/jcfr/Projects/Slicer-Debug/ITK-build/lib/./libITKCommon-5.3.so.1 [0]: normal symbol `itk::Object::New()'

And here the expected symbol that should be resolved against:

$ readelf --wide -s _ITKCommonPython.so  | c++filt  | ack "itk::Object::New"
 13688: 0000000002d9f8a0   315 FUNC    GLOBAL DEFAULT    9 itk::Object::New()

For more details, see The LD_DEBUG environment variable | B. Nikolic Software and Computing Blog

Proposed solution

Similarly to the namespace support available in Qt (See qtnamespacemacros.h), we are considering introducing macros like the following:

#define ITK_NAMESPACE itk  // default namespace that can be configured
#define ITK_BEGIN_NAMESPACE  namespace ITK_NAMESPACE {
#define ITK_END_NAMESPACE }

Consider solution

We also considered creating a monolithic itk module (e.g _ITKPython.so) but that would increase its size and would lead to similar issue for ITK based modules.

Next steps

The ITK fork integrated in Slicer will be used to test and develop the namespace support.

We will also plan to provide scripts and instruction to (1) update existing code and (2) developer ITK classes and (3) use ITK classes to be compatible with arbitrary value of ITK_NAMESPACE

2 Likes

This sounds like a good approach, and is common in many of ITK’s third party libraries.

Is there a known motivation for ITK_BEGIN_NAMESPACE and ITK_END_NAMESPACE? It seems like it may obscure things, and inhibit reality some.

Also most of the test are outside of the itk name space, so including “ITK_NAMESPACE” as a prefix is going to get rather verbose quickly.

Is there a known motivation for ITK_BEGIN_NAMESPACE and ITK_END_NAMESPACE?

This is inspired from what Qt has been doing.

References:

most of the test are outside of the itk name space

Code aiming to support using ITK built with a namespace different from itk would have to be updated.

For example, itkSampleTest4.cxx would have to be updated like this:

diff --git a/Modules/Numerics/Statistics/test/itkSampleTest4.cxx b/Modules/Numerics/Statistics/test/itkSampleTest4.cxx
index 866dded14e..4ded363948 100644
--- a/Modules/Numerics/Statistics/test/itkSampleTest4.cxx
+++ b/Modules/Numerics/Statistics/test/itkSampleTest4.cxx
@@ -21,7 +21,7 @@
 #include "itkObjectFactory.h"
 #include "itkMath.h"
 
-namespace itk
+namespace ITK_NAMESPACE
 {
 namespace Statistics
 {
@@ -132,7 +132,7 @@ itkSampleTest4(int, char *[])
 
   using MeasurementVectorType = std::vector<float>;
 
-  using SampleType = itk::Statistics::SampleTest::MySample<MeasurementVectorType>;
+  using SampleType = ITK_NAMESPACE::Statistics::SampleTest::MySample<MeasurementVectorType>;
 
   auto sample = SampleType::New();
 
@@ -175,7 +175,7 @@ itkSampleTest4(int, char *[])
 
   for (unsigned int j = 0; j < MeasurementVectorSize; ++j)
   {
-    if (itk::Math::NotExactlyEquals(measureBack[j], measure[j]))
+    if (ITK_NAMESPACE::Math::NotExactlyEquals(measureBack[j], measure[j]))
     {
       std::cerr << "Error in Set/Get MeasurementVector()" << std::endl;
       return EXIT_FAILURE;
@@ -191,7 +191,7 @@ itkSampleTest4(int, char *[])
     std::cerr << "Sample failed to throw an exception when calling SetMeasurementVectorSize()" << std::endl;
     return EXIT_FAILURE;
   }
-  catch (const itk::ExceptionObject & excp)
+  catch (const ITK_NAMESPACE::ExceptionObject & excp)
   {
     std::cout << "Expected exception caught: " << excp << std::endl;
   }
@@ -205,7 +205,7 @@ itkSampleTest4(int, char *[])
   {
     sample->SetMeasurementVectorSize(MeasurementVectorSize + 5);
   }
-  catch (const itk::ExceptionObject & excp)
+  catch (const ITK_NAMESPACE::ExceptionObject & excp)
   {
     std::cerr << excp << std::endl;
     return EXIT_FAILURE;

And since ITK already has a namespace, the introduction of ITK_BEGIN_NAMESPACE and ITK_END_NAMESPACE is likely not needed, and using only ITK_NAMESPACE macro (or an intermediate alias set based on the macro) would be sufficient.

1 Like

In looking into my own question I came upon this SO:

Which looks like one advantage of the BEGIN/END macro is defining nested namespace. However C++17 has improved the syntax to allow for defining nested names spaced with `::``, so the additional benefits are less significant.

Is there any friendlier alternative for the macro name? ITK_NAMESPACE seems extremely long for having a lot of copies of it in the code.

itkns maybe? having 4 consonant in a row seems a good protection against name collides out there in the wild.

1 Like

We should indeed have a way to optimize and reduce the verbosity by using something like this:

using itkns = ITK_NAMESPACE

or

#if !defined(ITK_NAMESPACE_IS_ITK)
using itk = ITK_NAMESPACE_IS_ITK;
#endif
1 Like