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).
Based on your finding and implemented fixes, could you add entry to https://www.slicer.org/wiki/Documentation/Nightly/Developers/Tutorials/MigrationGuide#Transition_from_ITK4_to_ITK5
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…
Upgrading external modules should be a matter of upgrading to ITKv5 new threads,
if the ThreadedGenerateData makes no use of
void ThreadedGenerateData( const OutputRegionType& threadRegion, ThreadIdType threadId )
void DynamicThreadedGenerateData( const OutputRegionType& threadRegion )
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:
#include <itkMultiThreader.h> itk::MultiThreader::Pointer ProcessingThreader;
#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…
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):
- 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.
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.
Looks like there’s a small conflict between @cpinter’s patch and the one @N-Dekker did. If it doesn’t get resolved in a few days then yes, doing temporary Slicer-specific elastix fork would be a good idea.
Just returned from travelling. I’ll take care of this tomorrow. Thanks for the patience
This pull request fixes the build issues (only tested on Windows) and adds node references from the transform to the inputs: