vnl_matrix_fixed build errors

My colleague @denis.p.shamonin and I observed link errors on an application linking to ITK, latest commit from the master branch: Merge topic 'ComputeJacobianPositionAsFixedType' · Kitware/ITK@8e82137 · GitHub

Visual C++ compiler output:

error LNK2019: unresolved external symbol “public: __cdecl vnl_svd_fixed<double,2,2>::vnl_svd_fixed<double,2,2>(class vnl_matrix_fixed<double,2,2> const &,double)” (??0?$vnl_svd_fixed@N$01$01@@QEAA@AEBV?$vnl_matrix_fixed@N$01$01@@N@Z) referenced in function "public: virtual void __cdecl itk::Transform<double,2,2>::ComputeInverseJacobianWithRespectToPosition(class itk::Point<double,2> const &,class vnl_matrix_fixed<double,2,2> &)const " (?ComputeInverseJacobianWithRespectToPosition@?$Transform@N$01$01@itk@@UEBAXAEBV?$Point@N$01@2@AEAV?$vnl_matrix_fixed@N$01$01@@@Z)

error LNK2019: unresolved external symbol "public: class vnl_matrix_fixed<double,2,2> __cdecl vnl_svd_fixed<double,2,2>::pinverse(unsigned int)const " (?pinverse@?$vnl_svd_fixed@N$01$01@@QEBA?AV?$vnl_matrix_fixed@N$01$01@@I@Z) referenced in function "public: class vnl_matrix_fixed<double,2,2> __cdecl vnl_svd_fixed<double,2,2>::inverse(void)const " (?inverse@?$vnl_svd_fixed@N$01$01@@QEBA?AV?$vnl_matrix_fixed@N$01$01@@XZ)

It appears reproducible as follows:

// ITK user application (linking to ITK):
#include <itkAffineTransform.h>

int main()
{
  itk::AffineTransform<double, 2>::New();
}

A workaround (quick-fix) for ITK users could be to add two #include statements:

// ITK user application (linking to ITK):
#include <vnl/algo/vnl_svd_fixed.hxx>  // Quick-fix
#include <vnl/vnl_fortran_copy_fixed.hxx>  // Quick-fix
#include <itkAffineTransform.h>

int main()
{
  itk::AffineTransform<double, 2>::New();
}

@hjmjohnson, @blowekamp Do you have any clue what’s going on? See also http://review.source.kitware.com/#/c/23657/8

For the record, the quick-fix that I suggested to work around those vnl_matrix_fixed link errors triggered some other compile errors, when building a MeVisLab module:

#include <mlModuleIncludes.h> // For MeVisLab (MeVisLab3.0.2VC14-64)
#include <vnl/algo/vnl_svd_fixed.hxx>  // Quick-fix to avoid link errors
#include <vnl/vnl_fortran_copy_fixed.hxx>  // Quick-fix to avoid link errors

Triggered Visual C++ compile errors:

ITK\modules\thirdparty\vnl\src\vxl\v3p\netlib\laso/dnlaso.h(28): error C2144: syntax error: ‘char’ should be preceded by ‘)’
ITK\modules\thirdparty\vnl\src\vxl\v3p\netlib\laso/dnlaso.h(28): error C2144: syntax error: ‘char’ should be preceded by ‘;’
ITK\modules\thirdparty\vnl\src\vxl\v3p\netlib\laso/dnlaso.h(28): warning C4091: ‘’: ignored on left of ‘char’ when no variable is declared

This appears caused by a conflict between a #define small char in C:\Program Files (x86)\Windows Kits\8.1\Include\shared\rpcndr.h, and a function declaration in ITK\modules\thirdparty\vnl\src\vxl\v3p\netlib\laso\dnlaso.h:

extern int v3p_netlib_dnwla_(
  ....
  v3p_netlib_logical *small,  // Mind the 'small' identifier!!! -- Niels
  v3p_netlib_logical *raritz,
  v3p_netlib_doublereal *delta,
  v3p_netlib_doublereal *eps,
  v3p_netlib_integer *ierr
  );

But clearly that’s unrelated to the original link errors.

Niels,

It is strange that your addition of pre-processor commands fixed a link error.

I’m having a very difficult time replicating the problem. Could you make an zip file with a CMakeLists.txt file and the source file that causes the problem? I’ve been unable to generate this issue.

Hans

Thanks for your reply, and your attempt to reproduce the link errors, @hjmjohnson Now I see, it was my own fault :roll_eyes: My CMakeLists.txt did not link to all libraries from ${ITK_LIBRARIES}. Instead I just listed the subset of ITK lib files that I thought my project really needed:

target_link_libraries(MyProject 
  ${ITK_LIB_DIR}/ITKCommon-5.0.lib
  ${ITK_LIB_DIR}/itkvnl-5.0.lib
  ${ITK_LIB_DIR}/itkvnl_algo-5.0.lib
  ${ITK_LIB_DIR}/itksys-5.0.lib
  ${ITK_LIB_DIR}/itkv3p_netlib-5.0.lib
  ${ITK_LIB_DIR}/itkvcl-5.0.lib
)

Apparently I should add ITKVNLInstantiation-5.0.lib, or just use ${ITK_LIBRARIES}! Either way, my link errors are gone now. :grinning:

For the future, if you just want a subset of ITK, point to the required modules using COMPONENTS, and then ${ITK_LIBRARIES} will only contain this subset. For example:

find_package(ITK REQUIRED COMPONENTS
  ITKCommon
  ITKIOImageBase
  ITKImageGrid
  ITKImageIntensity
  ITKImageStatistics
  ITKImageIO
  CONFIG
)
include(${ITK_USE_FILE})
target_link_libraries(Foo PUBLIC ${ITK_LIBRARIES})

You can check what modules a particular .cpp requires using Utilities/Maintenance/WhatModulesITK.py

2 Likes

No problem! We’ve all been there.

Hans

1 Like

Interesting! I’ll have a closer look.

But anyway, for the lime being I still maintain a list of those ITK lib files for my MeVisLab project, which is not using CMake (yet). MeVisLab uses Qt/QMake, instead of CMake, to create Visual Studio project files. Which means that I have to list the lib files in a QMake PRO file.

The same commit caused link errors in the RTK code, see RTK dashboard, e.g., this build. vnl_svd_fixed is indeed not instantiated for 1D registration, which we use in our code (here). Is this the expected behavior? If yes, should I instantiate the missing classes on RTK side?

@simon.rit based on your code, I added a test, reproduced the build errors, and fixed them in this patch:

http://review.source.kitware.com/#/c/23687/

Thanks a lot, we are back to a clean RTK dashboard. If I understand well, it seems that you are now favoring performances over genericity. Understandable but for the lambda user, should we explicitly state in the documentation or with macros which dimensions can be instantiated?

As a farmer I know would say,

That’s a thing of friggin’ beauty.

I think we strive to be both performant and generic.

Good idea :bulb:

Currently, any transform dimension should be supported in general, but only dimensions 1 to 9 are supported for transform IO.

There was a recent change in the itk::Transform classes which changed the VNL SVD computation from using dynamically sized arrays to fixed arrays. The fixed SVD changes was proposed as a WIP, but it got mixed up in the topic which had other significant performance enhancements for computing the Jacobian in the registration framework:

ITK’s VNL library configuration requires explicit instantiations for several types to use the fixed SVD:

These instances are not complete for 1-9, additionally there is the possibility to create transforms from N->M dimensions so the permutations need to be considered.

There are several other places in ITK which are using the dynamically sized SVD algorithm. This could use further investigation When this is done on a per pixel basis the dynamic allocation can be a significant scalability and performance bottleneck. ( When optimizing a filter/algorithm removing dynamic allocation in the inner loops is one of the first things to look at ). The broader use of fixed SVD in ITK could use further investigation.

A simular issue with itk::Matrix has occurred before:

With the current trend in ITK of trying to instantiate ITK for many types and dimensions for wrapping, the burden of having the code in headers only and not in compile object files is apparent. The bloated object files with duplicate compiled code is apparent. Explicitly instantiating code provide compile and binary size efficiency, so it is something we should strive to do more in ITK.

What are the options:

  1. Instantiate more VNL types ( perhaps creating a macro to instantiate multiple classes as once )
  2. Revet to the old dynamic SVD code, and not have the opportunity to use fixed SVD in ITK, but the SVD code in the Transform class is not used much.
  3. Do some meta-programming one the transform dimensions if N <= 5 && M==N then use fix SVD else use dynamic.
  4. updated Document that additional vnl headers should be included when the transform dimensions are not instantiated, however the current VNL instantiation are not in headers so library instantiated templates will be re instantiated.
1 Like

While improved explicit instantiation support would help the build time and binary size of Python bindings and other large applications, we should not be striving, in general, for forced, explicit instantiation.

An optional explicit instantiation, as described here:

would be a nice feature.

More explicit instantiation increases build complexity, build times, and build size for common use cases. If an application is developed using templates, only the template instantiations that are used are compiled. You not need to compile and link instantiations that are not used.

In my ITK build, libITKTransformFactory-5.0.so.1 followed by libitkvnl-5.0.so.1 are the largest libraries with a whopping 5.8 MB and 4.7 MB, respectively, even for an -Os / MinSizeRel build. In a real application, are both float and double transform’s of all different types for dimension 1-9 used? Rarely if ever.

In particular, it is extremely important that we do not build and link unused code when generating WebAssembly binaries. This impacts binary size, which is critical for WebAssembly. The ITKCommon and itkvnl libraries need to go on a diet for this use case. :canned_food:

An approach of fixed size implementations for common sizes and dynamic implementations for larger sizes provides a good balance of performance and binary size. Fixed sizes that are always used could be explicitly instantiated.

Build errors related to vnl_matrix_fixed are also reported for ANTs here:

These build errors seems to be related. Can anyone confirm? Or fix?

Edit: this might be due to some CMake/Ninja hiccup. Let’s wait for tomorrow’s or Monday’s results.