ENH: Modernize TractographyTRX Eigen includes

[mattcieslak]

PR Checklist

• No API changes were made (or the changes have been approved)
• No major design changes were made (or the changes have been approved)
• Added test (or behavior not changed)
• Updated API documentation (or API not changed)
• Added license to new files (if any)
• Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
• Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

Sorry for all the PRs, hoping this will be the last one for awhile. Following up on #5951 I changed how Eigen is included in the TractographyTRX module to take advantage of #5831. I added some extra CI tests so hopefully @dzenanz won't keep finding surprises when building locally. This includes changes to both trx-cpp and ITKTractographyTRX to make Eigen-finding more robust.

[thewtex]

@mattcieslak thank you!

[dzenanz]

Incremental build failed to configure for me:

CMake Error at C:/Misc/ITKTractographyTRX-vs22/_deps/trx_cpp-src/CMakeLists.txt:131 (TARGET_LINK_LIBRARIES):
  Target "trx" links to:

    Eigen3::Eigen

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.

Doing a clean build does not help.

[mattcieslak]

Doing a clean build does not help.

Do you have a recommendation on how to catch this in a CI build? I'm using the InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction and it's building there and passing tests. I feel bad wasting your time with non-compiling PRs

[mattcieslak]

@dzenanz are you building locally against an already-built ITK?

[blowekamp]

I don't think the ITKRemoteModuleBuildTestPackageAction infrastructure has been updated since 6.0b01, there is the 6.0b02 that is in the process of being release. I presume the @thewtex is working on that tag. This update add the CMake interface modules. Where ITK::ITKEigenModule was introduced.

After this was the Eigen refactor which was merged on Friday, after the beta tag.

Just using "ITK::ITKEigenModule" as an interface library will work now, I may work for the 6.0b2 as well.

[dzenanz]

I created tee-ar-ex/ITKTractographyTRX#21 in an attempt to use the latest main version, which includes the Eigen refactoring.

Yes, I was building locally against an already-built ITK.

[thewtex]

I don't think the ITKRemoteModuleBuildTestPackageAction infrastructure has been updated since 6.0b01, there is the 6.0b02 that is in the process of being release. I presume the @thewtex is working on that tag.

Yes, I am testing now. I will follow-up when available.

[thewtex]

I will follow-up when available.

It is now available. However, the most recent ITK::ITKEigenModule changes are not in the default tag. They will come in the next release.

[blowekamp]

It is now available. However, the most recent ITK::ITKEigenModule changes are not in the default tag. They will come in the next release.

The Eigen3 interface library is only couple include paths and no compiled code, maybe some compilation definitions. I just check the 6.0b02 tag, and it looks like the include paths should be associated with ITK::ITKEigenModule. I think it's worth trying removing the find Eigen3 package code and just relying on the mentioned interface library.

[mattcieslak]

Should a remote module try to support older ITKs? Or is it assumed that it will only work with the version it's bundled with?

[dzenanz]

A remote module totally can support multiple versions of ITK. And right now is particularly reasonable time to do that. ITK 5 is stable, and ITK 6 on the horizon.

[mattcieslak]

Ok, I've got it building and passing tests on 5.4.5, main and release. should be good to go!

[blowekamp]

The goal of the updates for Eigen was to simplify the usage, in ITK and modules. Your code to use Eigen has gotten more complicated.

Your goal is to have one branch work for 5.x and the 6.x-development? This may be creating additional complexities that will cause maintenance problems in the future.

[dzenanz]

Perhaps you can suggest a way to simplify eigen-related CMake code?

[blowekamp]

What is the current goal? To get a python package for 6.0b2? or against the 5.4.5? Or building against the current main as part of the ITK build?

NOTE: I don't think the CI for remote modules support external build against the current main.

[mattcieslak]

I mostly want the module to be a good ITK citizen, so am open to either moving to the new Eigen include system exclusively or keeping the complicated multi-version system in there. My personal goal is to get TRX support in ANTs (ANTsX/ANTs#1947), but I'd also like this module to be useful to others. Happy to follow your suggestion here!

[blowekamp]

I made a PR: tee-ar-ex/ITKTractographyTRX#23

This is the simplified version to work with post 6.0b02 update to Eigen.

[mattcieslak]

Thanks again @blowekamp and @dzenanz for your help on this. I think this is ready to go now

[dzenanz]

This builds locally against a recent main.