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

Invalid terminology information in tooltip and Slicer crash using segment descriptor file context #135

Closed
che85 opened this issue Jan 24, 2017 · 80 comments

Comments

@che85
Copy link
Member

che85 commented Jan 24, 2017

Terminology seems not to work properly anymore when LoadTerminologyFromSegmentDescriptorFile/LoadAnatomicContextFromSegmentDescriptorFile are used.

Hovering over a segment within the integrated Segment Editor displays "invalid terminology information"

image

Also, when creating new measurements and selecting the context which has been loaded from the segment descriptor file, Slicer crashes. [1]

This is not related to the changes I made in #132

My assumption is, that this is related to the changes made using rapidjson instead of jsoncpp.

@cpinter , @lassoan Any ideas?

[1] https://gist.github.com/che85/4d0a772ac5adc03958333c2a16f8f3a1

@che85
Copy link
Member Author

che85 commented Jan 25, 2017

@fedorov Any thoughts on this? It's essential for the paper...

@fedorov
Copy link
Member

fedorov commented Jan 25, 2017

I was hoping @cpinter and @lassoan will respond. Considering no response, can you investigate a bit deeper?

@che85
Copy link
Member Author

che85 commented Jan 25, 2017

I tested on nightly build 01/06/2017 with using the master dcmqi and master QuantitativeReporting. This worked for me which brings me to the conclusion, that it must be the integration of rapidjson

@lassoan
Copy link
Collaborator

lassoan commented Jan 25, 2017

I've just got to this now. Quite likely regression due to rapidjson. I'm investigating.

@lassoan
Copy link
Collaborator

lassoan commented Jan 25, 2017

I need step-by-step instructions to reproduce the problem (simply browsing/changing terminologies in Segment Editor works fine)

@che85
Copy link
Member Author

che85 commented Jan 25, 2017

  1. load a structured report from the DICOM browser
  2. open Quantitative Reporting
  3. select table
  4. hover over color icon and check if it works

@che85
Copy link
Member Author

che85 commented Jan 25, 2017

Dataset that I used is QIN-HEADNECK-01-0139 from http://slicer.kitware.com/midas3/folder/3771

@fedorov
Copy link
Member

fedorov commented Jan 25, 2017

load a structured report from the DICOM browser

= in the DICOM browser select a series with modality SR from that study that Christian mentioned above

@lassoan
Copy link
Collaborator

lassoan commented Jan 25, 2017

ok, thanks!

@cpinter
Copy link
Collaborator

cpinter commented Jan 25, 2017

I also just got the notification. I'll look at the issue with the data you linked.

@fedorov
Copy link
Member

fedorov commented Jan 25, 2017

Thanks guys!

@cpinter
Copy link
Collaborator

cpinter commented Jan 25, 2017

Although the email header says 6PM yesterday, I'm pretty sure I just got it. Not sure why this is.
So in the future if something is urgent, probably sending a direct email too is a good idea, just in case.

@cpinter
Copy link
Collaborator

cpinter commented Jan 25, 2017

Sorry for the delay, I needed to rebuild parts of Slicer after updating. I also updated and built DCMQI and QuantitativeReporting.

When I load the SR series, I get this error
Traceback (most recent call last): File "C:/d/S4R/Slicer-build/lib/Slicer-4.7/qt-scripted-modules\DICOMLib\DICOMWidgets.py", line 790, in loadCheckedLoadables self.proceedWithReferencedLoadablesSelection() File "C:/d/S4R/Slicer-build/lib/Slicer-4.7/qt-scripted-modules\DICOMLib\DICOMWidgets.py", line 840, in proceedWithReferencedLoadablesSelection if not plugin.load(loadable): File "c:/d/_Other/QuantitativeReporting_R/lib/Slicer-4.7/qt-scripted-modules/DICOMTID1500Plugin.py", line 121, in load rwvmPlugin = slicer.modules.dicomPlugins["DICOMRWVMPlugin"]() KeyError: 'DICOMRWVMPlugin'

Any ideas what to do?

@fedorov
Copy link
Member

fedorov commented Jan 25, 2017

Csaba, we introduced a new dependency, which would be resolved if you installed via ExtensionManager.

You will need to build and add to the paths this extension: https://github.com/QIICR/Slicer-PETDICOMExtension

@cpinter
Copy link
Collaborator

cpinter commented Jan 25, 2017

Good to know, thanks!

Is it intentional that the name of this extension has "Slicer-" prefix and "Extension" postfix, while the others have neither? (QuantitativeReporting, LongitudinalPETCT, etc)

@cpinter
Copy link
Collaborator

cpinter commented Jan 25, 2017

After downloading and building the other extension I got past the previous error.
However I get another one, and I'm pretty sure it's not related to the original issue:

Traceback (most recent call last): File "C:/d/S4R/Slicer-build/lib/Slicer-4.7/qt-scripted-modules\DICOMLib\DICOMWidgets.py", line 790, in loadCheckedLoadables self.proceedWithReferencedLoadablesSelection() File "C:/d/S4R/Slicer-build/lib/Slicer-4.7/qt-scripted-modules\DICOMLib\DICOMWidgets.py", line 840, in proceedWithReferencedLoadablesSelection if not plugin.load(loadable): File "c:/d/_Other/QuantitativeReporting_R/lib/Slicer-4.7/qt-scripted-modules/DICOMTID1500Plugin.py", line 203, in load segmentationNodeID = segmentationNodes.GetItemAsObject(segmentationNodes.GetNumberOfItems()-1).GetID() AttributeError: 'NoneType' object has no attribute 'GetID'

@lassoan
Copy link
Collaborator

lassoan commented Jan 25, 2017

I've installed PETDICOMExtension and the issue is the same.

@che85
Copy link
Member Author

che85 commented Jan 25, 2017

@cpinter That probably means, that the Segmentation is not properly loaded into Slicer. Are you using the most recent version of QuantitativeReporting and dcmqi?

@cpinter
Copy link
Collaborator

cpinter commented Jan 25, 2017

Yes

@che85
Copy link
Member Author

che85 commented Jan 25, 2017

Other than that, there should be any error output in the log... or the segmentation itself is missing in the SlicerDICOMDatabase

@fedorov
Copy link
Member

fedorov commented Jan 25, 2017

@che85 could this be related to QIICR/dcmqi#173? I have not tested the latest one, but it worked for me in earlier nightlies.

@fedorov
Copy link
Member

fedorov commented Jan 25, 2017

Is it intentional that the name of this extension has "Slicer-" prefix and "Extension" postfix, while the others have neither? (QuantitativeReporting, LongitudinalPETCT, etc)

@cpinter that extension was developed by the Iowa team, and they follow their own naming conventions. I agree, I don't see a need for those. Maybe @chribaue can comment on this.

@che85
Copy link
Member Author

che85 commented Jan 25, 2017

@fedorov no that cannot be related since QIICR/dcmqi#173 is only about the webapps

@cpinter
Copy link
Collaborator

cpinter commented Jan 25, 2017

I imported the whole folder into the database, so this should not be the problem
image

I get this in the log also:
"SEG2NRRD did not complete successfully, unable to load DICOM Segmentation"
"RWVM is referenced from SR, but is not found in the DICOM database!"
The second one makes no sense, as it is in the DB.

@fedorov
Copy link
Member

fedorov commented Jan 25, 2017

The second one makes no sense, as it is in the DB.

If it is the same one that is referenced from SEG ... I will download the nightly and test from scratch now. Building is a slower option, since my slicer build is quite old.

@fedorov
Copy link
Member

fedorov commented Jan 25, 2017

@che85 sorry - I meant this one QIICR/dcmqi#170. Maybe in the process of changing the attribute names something was broken?

@che85
Copy link
Member Author

che85 commented Jan 25, 2017

@fedorov No I am pretty sure that it doesn't have to do with that since I tested the new dcmqi and Quantitative Reporting with an older nightly of Slicer from 01/06/2017 before integration of rapidjson

@che85
Copy link
Member Author

che85 commented Jan 25, 2017

@cpinter @fedorov @lassoan I am getting the same error with the latest nightly. I will look into the error messages

Traceback (most recent call last):
  File "/Applications/Slicer 2.app/Contents/lib/Slicer-4.7/qt-scripted-modules/DICOMLib/DICOMWidgets.py", line 790, in loadCheckedLoadables
    self.proceedWithReferencedLoadablesSelection()
  File "/Applications/Slicer 2.app/Contents/lib/Slicer-4.7/qt-scripted-modules/DICOMLib/DICOMWidgets.py", line 840, in proceedWithReferencedLoadablesSelection
    if not plugin.load(loadable):
  File "/Applications/Slicer 2.app/Contents/Extensions-25669/QuantitativeReporting/lib/Slicer-4.7/qt-scripted-modules/DICOMTID1500Plugin.py", line 203, in load
    segmentationNodeID = segmentationNodes.GetItemAsObject(segmentationNodes.GetNumberOfItems()-1).GetID()
AttributeError: 'NoneType' object has no attribute 'GetID'

@fedorov
Copy link
Member

fedorov commented Jan 27, 2017

I thought this used to work ... let me think what I can turn off/change ...

@cpinter
Copy link
Collaborator

cpinter commented Jan 27, 2017

Strange. Unfortunately the CircleCI check also doesn't work any more (the last 15 or so checks failed the same way although all of them except my commit yesterday should have passed). Maybe we can ask Steve or someone to try to build it as well. I understand it's really urgent to make this work, and Christian only has Mac.

FYI I committed another small fix, it should be good now. There is only one very minor issue, namely that when entering QuantitativeReporting after loading the SR I get this warning many times:
Generic Warning: In C:\d\Slicer4\Modules\Loadable\Terminologies\Logic\vtkSlicerTerminologiesModuleLogic.cxx, line 434 PopulateTerminologyTypeFromJson: Unsupported data type for recommendedDisplayRGBValue
@che85 Do you remember seeing these warnings with the last version that worked, or do you think it's new? I cannot reproduce it without the QuantitativeReporting stuff, and SimpleITK doesn't build in debug on Windows, and in Release there is only so much I can do. Apart from this warning message it seems to work well now.

@fedorov
Copy link
Member

fedorov commented Jan 27, 2017

I have no idea what is going on with my build. I checked the manual, and there are no special provisions re SimpleITK on Mac. @che85 can you run a build on your system? Maybe it is because my OS is couple of generations old. Although, I know my build was working recently.

@cpinter
Copy link
Collaborator

cpinter commented Jan 27, 2017

I got similar ITK-related errors about but when building CTK. I also skipped OS updates, and I stopped being able to build Slicer a few months ago. I'm updating now to whatever animal or national park is the latest, then try from scratch. I won't be able to finish today for sure though.

@fedorov
Copy link
Member

fedorov commented Jan 27, 2017

I will update, but not before Feb 1 - too much risk of breaking things I will need for the paper!

@fedorov
Copy link
Member

fedorov commented Jan 27, 2017

Ah, I think I might have found it!

I think I actually did have this issue before. I will be trying this suggestion from Brad that worked last time:

There is the SimpleITK CMake options “SITK_EXPLICIT_INSTANTIATION”which could be set to OFF to solve this problem on your platform. Or maybe if you compile with SimpleITK static libraries it would not be an issue too.

Maybe not too late for you!

@cpinter
Copy link
Collaborator

cpinter commented Jan 27, 2017

It is :( However a build error should be quite easy to fix once you can reproduce. Although I hope the error is gone, as they seemed to be related to two warnings which I fixed since then.

@che85
Copy link
Member Author

che85 commented Jan 27, 2017

@cpinter I cannot recall if I have ever seen this error message.

@fedorov
Copy link
Member

fedorov commented Jan 27, 2017

@cpinter to make you feel better - that trick did not help!

@cpinter
Copy link
Collaborator

cpinter commented Jan 27, 2017

@che85 Thanks! I'm trying to fix it, but nevertheless, if the Mac build is OK then it will work now.

@lassoan
Copy link
Collaborator

lassoan commented Jan 27, 2017

It would be useful to limit SimpleITK dependency, as it's a huge component and the inability to debug makes troubleshooting almost impossible on Windows. Of course SimpleITK can be still used, but try to implement modules so that only those features are not available that actually require SimpleITK.

@cpinter
Copy link
Collaborator

cpinter commented Jan 27, 2017

@pieper If you can easily update and build Slicer on your Mac would you please try that? Thank you!

@che85
Copy link
Member Author

che85 commented Jan 27, 2017

I am also going to build Slicer from scratch now on my Mac.

@che85
Copy link
Member Author

che85 commented Jan 27, 2017

I was able to build Slicer without problems from Slicer/Slicer@707ee80

@pieper
Copy link
Member

pieper commented Jan 27, 2017 via email

@pieper
Copy link
Member

pieper commented Jan 28, 2017 via email

@cpinter
Copy link
Collaborator

cpinter commented Jan 28, 2017

That's great news, thanks!

@che85 could also build, so he can test the rest with the proper dataset and extensions.
I was worried it didn't build because the error message didn't make much sense to me, and was not sure if my commit actually fixed it.

@che85
Copy link
Member Author

che85 commented Jan 28, 2017

@cpinter no I cannot test it with my Slicer build since Slicer is not starting properly on my Mac OS Sierra. Will have to wait for the next nightly build.

@cpinter
Copy link
Collaborator

cpinter commented Jan 28, 2017

@che85 Do you know why it doesn't start? This is not what I'd call built without problems.

@che85
Copy link
Member Author

che85 commented Jan 28, 2017

@cpinter Slicer built 100% without errors. My startup problems already existed months ago...

@che85
Copy link
Member Author

che85 commented Jan 28, 2017

Seems like the nightly failed on the build system for Mac: http://slicer.cdash.org/viewBuildError.php?buildid=962482

Interesting that neither @pieper nor my build failed

@lassoan
Copy link
Collaborator

lassoan commented Jan 28, 2017

It seems to be a ConstMemberIterator / MemberIterator mismatch.

Solution is probably to replace RemoveMember by EraseMember as suggested here: Tencent/rapidjson#721

@cpinter Can you give this potential solution a try? (if you cannot test it yourself then just change it and wait for the next nightly)

@cpinter
Copy link
Collaborator

cpinter commented Jan 28, 2017

I replaced all RemoveMember calls to EraseMember, and it seems to work OK.
I cannot test Mac build (and apparently Christian and Steve cannot either as it succeeded for them before), so we'll see how the nightly goes.

@fedorov
Copy link
Member

fedorov commented Jan 28, 2017

I built Slicer with SimpleITK disabled, in Release mode. Loading works fine, and I see segmentations overlayed. I cannot access QuantitativeReporting UI, since it has a dependency on SimpleITK, but when I try to switch to SegmentEditor, I get a crash.

I will now build in Debug to see if I can get any more info on the crash.

@fedorov
Copy link
Member

fedorov commented Jan 28, 2017

Ok, I am super confused now - now it is working! It was crashing repeatedly, then I discovered it was actually build in debug mode (I guess the flag got reset as I was rebuilding), so I ran it in the debugger, it didn't crash, and now I cannot reproduce the problem. Hope I was just messing something up earlier - fingers crossed for tomorrow nightly!

@cpinter
Copy link
Collaborator

cpinter commented Jan 28, 2017

Thanks for the news, Andrey! We're actually late from a party now, so I'll then stop trying to reproduce and will get back to it in the evening. I hope it's not broken!

@fedorov
Copy link
Member

fedorov commented Jan 28, 2017

Csaba, definitely - it makes a lot of sense to wait until tomorrow. Thanks for your efforts!

@cpinter
Copy link
Collaborator

cpinter commented Jan 29, 2017

Mac build was successful on the factory, so that's good.
I tried reproducing the crash with Segment Editor, but I was unsuccessful.
Let me know if there are further issues.

@fedorov
Copy link
Member

fedorov commented Jan 29, 2017

Confirmed, looks good - thanks!!

@fedorov fedorov closed this as completed Jan 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants