-
Notifications
You must be signed in to change notification settings - Fork 47
ITK 5.2 upgrade #378
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
ITK 5.2 upgrade #378
Conversation
A WIP is in my fork: https://github.com/dzenanz/Seg3D/tree/itk5migration. I will continue working on this tomorrow. I have some unrelated link errors for boost, such as |
I am stuck at this possible bug in ITK. The boost link errors are gone in RelWithDebInfo build. The only other build error I have is related to HDF5:
Pointing to ITK's build directory instead of install might help with that. @allywarner do you get the same errors? |
@dzenanz Debug mode is supposed to work!I haven't seen that error before. Were you able to fix it? The other build error in reference to HDF5 is the error I'm gettting trying to build with ITK5. We can also look at my or your build together if you like! |
Incorporating the changes from @dzenanz (who incorporated changes from @allywarner), I have a WIP at Leengit/Seg3D branch itk5migration. There I was able to address a number of build-stopping errors (using gcc-8 on Ubuntu 20.04):
Outstanding is only the HDF5 error reported above. I believe that the include directory tree includes the path
... in a situation where it should be looking to
... but I haven't yet figured out how to fix that. |
There are some more errors on Visual Studio. Log from current WIP:
|
@dzenanz It's common to have many errors when upgrading Seg3D to a new ITK version. The error I'm most concerned about at the moment is the HDF5 error. |
I have added commits to Leengit/Seg3D branch itk5migration to address more ITK upgrade issues. @dzenanz is making changes to ITK (via the enableH5HL branch) to address the H5HL problems. |
I would like to contribute the commits on my Leengit/Seg3D branch itk5migration. Given this pull request already in progress and several branches owned by different parties, I don'tk now how to do that. Please advise. |
@Leengit When we get somebody's fork building, it is easy to have that person make a PR. For now, we can just cherry-pick each other's commits as we have done so far. With my fork, I get past
|
@dzenanz, the error you are seeing means that my fix to use |
My fork now build successfully. But speedline tool does not seem to work (it works in 2.2.1 I downloaded from the website). It looks like it was refactored since, but I cannot get it to work. Maybe I don't know how to use it? Or it was broken by fixes to priority queue in ITK? |
@dzenanz @jessdtate What happens when you try to use speedline? |
The fix to |
@Leengit it builds for me too. @allywarner when I click "Fill", there is no segmentation appearing. Also when I place control points, the line does not appear. |
@dzenanz I'll build your fork and test it out. |
@dzenanz I've done some testing on OSX 10.14.6 and I was able to get control points, but not able to get a segmentation. I'm going to take a look into it this week! |
Any updates @allywarner? |
@allywarner, feedback? |
@dzenanz, The ExtractImageFilter needed SetDirectionCollapseToStrategy() to be able to move forward! There was a lousy try catch in Seg3D that wasn't throwing any exception at all. So I've set that and so far it has the same behavior as the release version. For me, the release version of Speedline doesn't work on my machine (Windows and OSX 10.14.6). @jessdtate says that it works for him on OSX 10.12. Speedline is using Livewire segmentation and it claims that the anchor is outside of the region therefore it's not calculating any paths between the selected points. |
@dzenanz It does work! Wahoo! I do have some SQL errors though so I'm going to look through those. |
Set Direction Collapse to Identity
@dzenanz this PR should build and not have errors as far as I know. Will the H5HL branch be merged into master? |
case Core::DataType::UINT_E: | ||
buffer_size = sizeof( int ); | ||
break; | ||
case Core::DataType::LONGLONG_E: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least, this commit should be called Dženan's and Lee's changes
. But I prefer you cherry-picking all our commits with their messages (possibly squashing some, but not all commits).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I'm so sorry! I didn't mean to not give credit to Lee! I really appreciate the help. I will cherry pick in the future!
ENDIF() | ||
|
||
SET(itk_GIT_TAG "origin/itk5.1.0") | ||
SET(itk_GIT_TAG "enableH5HL") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just merged the commit from H5HL branch into master, so this should be changed to master
until v5.2.0 is tagged.
"-DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR>" | ||
"-DITK_BUILD_DEFAULT_MODULES:BOOL=OFF" | ||
"-DITKV3_COMPATIBILITY:BOOL=OFF" | ||
"-DITKV4_COMPATIBILITY:BOOL=ON" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The combination of settings here is indicative of full update to ITKv4, and minimal update to ITKv5. For future proofing, we should finish migration to ITKv5. That should be done in a new PR. Possibly after allowing a week or so of use to see if there are any immediate bugs/changes caused by this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turning on removal of legacy code did not require further changes on my machine. I created an alternative PR with full commit history in #382.
Updating Seg3D to ITK 5.1.0 to fix bugs.