SlicerRT does not build due to missing ITK remote module

Thanks a lot! I’ll try to do that

I checked the lib directory again, and found an ‘ITKBioCell-5.0.lib’. The reason it was not found was that the project wanted to find an ‘ITKBioCell.lib’. I’ll work with @gcsharp to fix the Plastimatch build within SlicerRT, and then probably issue a PR to include this remote.
Thanks!

Unfortunately there are multiple other errors too when building Plastimatch with ITKv5.

I wish some more testing had been done before making the switch, at least trying to build the most downloaded extensions. Now we have to scrape to have a working SlicerRT, which is by far the most downloaded extension. I already got emails about why it is not available. This is an emergency for us that I think would have been avoidable.

For others struggling with the same thing, here is the ITK 5 migration guide: https://github.com/InsightSoftwareConsortium/ITK/blob/master/Documentation/ITK5MigrationGuide.md

1 Like

Nightly Slicer will be unstable for a while due to many major changes that we are planning to do (Python3…). Can your users stay on the latest stable version?

That’s what I told them. There are not a lot of changes in SlicerRT so they don’t lose much. But we might lose users who try the nightly and they don’t find what they want.

3 posts were split to a new topic: Improve description of nightly build on download.slicer.org

After taking a better look, these errors, although there are many, do not seem hard to fix. At least for us what took the long time was to investigate what broke exactly and to find the time to start the fixes (deadlines etc). Doing the fixes should be now easy, and can be expected in a day or two.

I don’t think it is needed to revert the change unless there are multiple other extensions that rely heavily on ITK as much as SlicerRT (due to Plastimatch mainly).

1 Like

Based on your finding and implemented fixes, could you add entry to Documentation/Nightly/Developers/Tutorials/MigrationGuide - Slicer Wiki

A post was split to a new topic: Streamlining maintenance of download.slicer.org

Some of the things are specific to Plastimatch (e.g. I don’t think anyone else uses BioCell), and I find that ITK migration guide is pretty good. Since there is a link already to the full migration guide I think it is enough.

I just saw this post about release of ITK 4.13.2, which made me quite confused about the discussion in this thread. Quoting that post,

The next feature pre-release for ITK 5, ITK 5 Release Candidate 2, is anticipated in a few weeks

If ITK 5 is not yet released, we only have release candidate 1, why is Slicer switching to ITK 5 now?

Part of the motivation was that @phcerdan was available to do the troubleshooting for the needed C++ changes. Also this has been on the roadmap for a long time to upgrade all the dependencies and we planned that the nightly would be undergoing a lot of slicer5 changes (python3 is coming soon too) and that was accepted since we have a 4.10.1 already and plans for a 4.10.2 soon for people who aren’t ready to change. I think it’s fine that we are a bit ahead of the game on ITK. I doubt it’ll change much between RC and release.

I think C++ changes is different from upgrading to a dependency that is in a pre-release stage. I did not find ITK 5 upgrade in the roadmap, maybe I missed it. Maybe it all makes sense to the developers closely involved, but for someone a bit on the outside, switch to a prerelease of a key dependency looks strange.

Personally I don’t think it’s strange at all that the pre-release version of Slicer would be using the almost released version of ITK. You are right that the roadmap does not specifically callout ITK5, but it makes it clear that disruptive changes are coming…

Hi @fedorov, I updated the wiki with the steps taken on the transition. ITKv4->ITKv5 and the ITKv5 Migration Guide

Upgrading external modules should be a matter of upgrading to ITKv5 new threads,
if the ThreadedGenerateData makes no use of threadId replacing:

void ThreadedGenerateData( const OutputRegionType& threadRegion, ThreadIdType threadId )
void DynamicThreadedGenerateData( const OutputRegionType& threadRegion )

will do.

In other cases, where threadId is involved, the ITK migration guide has examples, but to just use the ITKv4 threading system add to the ITK class constructor:

this->DynamicMultiThreadingOff();

And replace

  #include <itkMultiThreader.h>

  itk::MultiThreader::Pointer ProcessingThreader;

with:

  #include <itkPlatformMultiThreader.h>

  itk::PlatformMultiThreader::Pointer ProcessingThreader;

that will probably solve 99% of the problems upgrading to ITKv5.

I am sorry the upgrade forced external plugins to catch up…

1 Like

I went through the extensions on the latest nightly dashboard that did not build. These are the extensions that fail to build due to ITK and the failure seems to be related to version 5 (and not some older issue like itkFactoryRegistration):

  • ABC
  • DSCMRIAnalysis
  • DTI-Reg
  • DTIAtlasBuilder
  • DTIProcess
  • IASEM
  • PET-IndiC
  • PETTumorSegmentation
  • PkModeling
  • ResampleDTIlogEuclidean
  • SkullStripper
  • SlicerElastix
  • SlicerVMTK
  • SPHARM
  • VirtualFractureReconstruction (also failed before for different reasons)

All of these issues are related to these few things:

  • Changed constants such as ITK_THREAD_RETURN_TYPE
  • The abovementioned multi-threading changes
  • Changed vnl functions (e.g. vnl_math_isinf or vnl_math_abs)
  • Deprecated vcl functions (e.g. vcl_cstdio.h or vcl_vector)
  • Changed image initialization ( conversion from ‘int’ to ‘const itk::SmartPointer<itk::Image<float, 3> >’ is ambiguous) - I haven’t met this one when fixing SlicerRT but I think this should be new as well

I volunteer fixing SlicerElastix as I’m planning to work on it regarding the node references I’m adding to all registration modules.

4 Likes

Thanks @cpinter! You probably know this, but @lassoan checked the elastix repo and they were actively porting to itk5 so maybe it’s just a matter of changing the superbuild hash.

1 Like

@chribaue @abeers FYI

Just an update that I issued a PR to Elastix to fix the remaining build errors. Once this is integrated I’ll update the hash.

This is great, thank you! Until they integrate the fix, you can fork the Elastix repository and use that fork for nightly builds. It would allow us to start testing Elastix build on all operating systems.