Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linking errors with shared libraries with GradientDescentOptimizerv4Template symbols #2838

Closed
blowekamp opened this issue Oct 26, 2021 · 10 comments · Fixed by #2848
Closed
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances

Comments

@blowekamp
Copy link
Member

blowekamp commented Oct 26, 2021

I am getting linking errors on windows with shared libraries with this change. Linking error such as:

ITKOptimizersv4-5.3.lib(ITKOptimizersv4-5.3.dll) : error LNK2005: "protected: virtual __cdecl itk::GradientDescentOptimizerv4Template<double>::~GradientDescentOptimizerv4Template<double>(void)" (??1?$GradientDescentOptimizerv4Template@N@itk@@MEAA@XZ) already defined in sitkImageRegistrationMethod.obj [D:\a\1\bld\SimpleITK-build\Code\Registration\src\SimpleITKRegistration.vcxproj] [D:\a\1\bld\SimpleITK.vcxproj]

This is occurring because LBFGS2Optimizerv4 is a non-templated class and is compiled into the shared library. However, GradientDescentOptimizerv4Template is a template class as it linkage is specified as such, but it is being implicitly compiled and included in the ITKOptimizersv4 library as a side effect of being the parent class of LBFGS2Optimizerv4.

I have my doubt that it would be worth the trouble (likely cause other problem) to explicitly instantiate the GradientDescentOptimizerv4Template into the library. Are there other suggestions?

The PR #2372 is the origins of this issue.

@blowekamp blowekamp added the type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances label Oct 26, 2021
@dzenanz
Copy link
Member

dzenanz commented Oct 26, 2021

Shouldn't identical duplicate template instantiations be ignored by the linker? How doesn't this problem arise in ITK's Python wrapping, which probably instantiates GradientDescentOptimizerv4Template?

@blowekamp
Copy link
Member Author

blowekamp commented Oct 26, 2021

Shouldn't identical duplicate template instantiations be ignored by the linker?

I think something like that happens on Linux and OS X, but not Windows which usually is more rigid with it's shared library symbols.

How doesn't this problem arise in ITK's Python wrapping, which probably instantiates GradientDescentOptimizerv4Template?

Does the ITK distributed binaries compile the ITK libraries as shared or static? I thought that one just one shared library (*.pyd) per ITK module?

@dzenanz
Copy link
Member

dzenanz commented Oct 26, 2021

I think they are compiled as shared libraries, by default one per module. Is SimpleITK different?

@blowekamp
Copy link
Member Author

I think they are compiled as shared libraries, by default one per module.

With ITK python there are not ITK shared libraries (e.g. ITKOptimizersv4-5.3.dll ) there are only the loadable ITK python modules (*.pyd). The C++ ITK library is compile as static without the exports.

Is SimpleITK different?

Yes. SimpleITK has compiled C++ libraries which use ITK's compiled C++ libraries.

@dzenanz
Copy link
Member

dzenanz commented Oct 27, 2021

I don't have a suggested solution. Matt might have an opinion or a suggestion. I will ping him via email.

@blowekamp
Copy link
Member Author

There are several parents in the hierarchy above the LBFGS2 Optimizer that are also templated. These would all need to be explicitly instantiate too which would be a bit of tedious work and refactoring. It may be best to make the LBFSG2 Optimizer a template and remove it from the compiled library.

While it would be a step backwards, reverting the change in parent class is an option too.

@dzenanz
Copy link
Member

dzenanz commented Oct 27, 2021

make the LBFSG2 Optimizer a template

This would be a backwards-incompatible change, right? Is SimpleITK compiled with legacy off? If so, your suggestion would work.

@thewtex
Copy link
Member

thewtex commented Oct 27, 2021

Yes, a non-templated class inheriting from a template-class is going to be problematic for shared symbols.

It may be best to make the LBFSG2 Optimizer a template and remove it from the compiled library.

This seems reasonable.

@blowekamp
Copy link
Member Author

Changing the base class as done in PR #2372 can be considered a backwards incompatible change.

Making LBFGS2 Optimizer a template would need to follow the same patter as done with the other optimizers. Some line similar to the following is used:

/** This helps to meet backward compatibility */
using GradientDescentOptimizerv4 = GradientDescentOptimizerv4Template<double>;

Where the original class name had the "Template" moniker added to the end, and an alias for the original class name.

@dzenanz
Copy link
Member

dzenanz commented Oct 27, 2021

Now I see that your suggested solution is not problematic at all. I guess that required change should be simpler than I thought too. Can you make a PR @blowekamp?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants