---
### Author:  [@lassoan](https://discourse.slicer.org/u/lassoan)
#### Post date: [May 21, 2017, 8:03pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/6 "2017-05-21T20:03:26Z")
The archetype reader was useful because it allowed a saved scene to refer to existing DICOM files. However, this does not work anymore (since the change that forces the file name to be the same as node name on save).
I think we should also consider David Gobbi’s [vtk-DICOM](https://github.com/dgobbi/vtk-dicom/) package. It is the most comprehensive open-soruce DICOM image reading/writing solution - it can read multiframe, cine, multidimensional, compressed, gantry-tilt images and it can also write several image IODs (see more info [here](http://dgobbi.github.io/vtk-dicom/doc/api/image_reader.html) and [here](http://dgobbi.github.io/vtk-dicom/)). It’s small, clean, simple, supported and continuously improved by David.
---
### Author:  [@pieper](https://discourse.slicer.org/u/pieper)
#### Post date: [May 21, 2017, 8:57pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/7 "2017-05-21T20:57:20Z")
vtkDICOM looks good to me as well.
The only downside I can see is that for some applications, like in dcmqi, we might end up duplicating some of the slice-to-volume conversion logic in something closer to native DCMTK without the VTK dependency.
---
### Author:  [@fedorov](https://discourse.slicer.org/u/fedorov)
#### Post date: [May 21, 2017, 9:43pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/8 "2017-05-21T21:43:02Z")
I think to expedite things and help the user, I should add an alternative scalar volume reader to the multivolume importer plugin, label its output loadable as “DCMTK scalar volume”, and use it in the multivolume either as the primary, or as a fallback when the first-order scalar volume plugin fails.
---
### Author:  [@pieper](https://discourse.slicer.org/u/pieper)
#### Post date: [May 21, 2017, 10:34pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/9 "2017-05-21T22:34:35Z")
Sounds very reasonable.
I wonder if you could add a testing mode where both loading mechanisms are used and the user (developer) is notified if they give different results.
---
### Author:  [@fedorov](https://discourse.slicer.org/u/fedorov)
#### Post date: [May 22, 2017, 6:32pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/10 "2017-05-22T18:32:16Z")
For what it’s worth:
- this branch replaces GDCM with DCMTK image IO: [https://github.com/fedorov/Slicer/tree/dcmtk-for-scalar-volume](https://github.com/fedorov/Slicer/tree/dcmtk-for-scalar-volume) - feel free to test any “corner cases”
- the build of the branch above works fine, and I can load the DCE series from [ERROR with DCE MRI loading in DICOM Browser](https://discourse.slicer.org/t/error-with-dce-mri-loading-in-dicom-browser/327/30) without problems
- almost all of the tests are passing when I tested on linux (from the list below, most of the failing tests failed because I killed Slicer app manually, as the test was running for very long time, often without any indication of what it was doing, some of them keep doing something or showed non-responsive app window after 10 minutes and up)
The following tests FAILED:
440 - py\_StandaloneEditorWidgetTest (Failed)
601 - py\_AtlasTests (Failed)
615 - py\_RSNAVisTutorial (Failed)
616 - py\_RSNAQuantTutorial (Failed)
622 - py\_RSNA2012ProstateDemo (Failed)
623 - py\_JRC2013Vis (Failed)
640 - py\_LandmarkRegistration (Failed)
Errors while running CTest
---
### Author:  [@fedorov](https://discourse.slicer.org/u/fedorov)
#### Post date: [May 30, 2017, 4:08pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/11 "2017-05-30T16:08:41Z")
We thought about this a bit more and discussed the implementation plan further with @pieper.
The following issues related to the initially considered approach of introducing a new C++ CLI that would read a DICOM series using DCMTKImageIO have been identified:
- there will be a need to re-implement the code that determines the scalar type from DICOM - this has to be done before the ITK reader is instantiated!
- since in the general case we cannot assume that the directory containing DICOM files does not include files from other series, and there is a limit on the maximum length of a command line in Windows, we would need to work around this by either copying files into a temp location, or communicating the list of files via a file
Based on this, and further analysis of the archetype reder code, an alternative approach has been developed:
- add a flag to ArchetypeScalarVolumeSeriesReader that would allow at runtime to select between GDCM and DCMTK image IO (done, see this commit in a branch referenced earlier: [https://github.com/fedorov/Slicer/commit/505265f8c1c48aea3ae3619e4718b5902170dafb#diff-849c98df12f92abb7d0eb4d31e9f83a5R100](https://github.com/fedorov/Slicer/commit/505265f8c1c48aea3ae3619e4718b5902170dafb#diff-849c98df12f92abb7d0eb4d31e9f83a5R100))
- propagate the capability of setting the flag to the layer that is used by DICOMScalarVolumePlugin (currently it is using Volumes logic
- keep GDCMImageIO as default to address the concerns expressed earlier about limited experience with DCMTKImageIO in Slicer
- in the case when loading an image series as a scalar volume fails in `load()` of the DICOMScalarVolumePlugin (`None` returned in this line: [https://github.com/Slicer/Slicer/blob/master/Modules/Scripted/DICOMPlugins/DICOMScalarVolumePlugin.py#L298](https://github.com/Slicer/Slicer/blob/master/Modules/Scripted/DICOMPlugins/DICOMScalarVolumePlugin.py#L298)) using GDCMImageIO (this is what was happening in the use case reported by the user that motivated this discussion), retry loading of the series using DCMTKImageIO
By using this approach, we will not change any of the behavior in the application or its reliance on GDCM, unless GDCM fails. As such, we will be able to introduce the fix addressing the user need relatively easily, and without waiting for the next release, and without confusing the user by introducing a new type of plugin for scalar volumes.
If you have any concerns or suggestions, please respond. We plan to proceed with the implementation of the proposed approach very soon.
---
### Author:  [@jcfr](https://discourse.slicer.org/u/jcfr)
#### Post date: [May 30, 2017, 4:51pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/12 "2017-05-30T16:51:54Z")
> done before the ITK reader is instantiated
May this could be added to ITK … in that case a list of readers could be returned. @thewtex Do you think that make sense or should Slicer team proceed as described ?
> or communicating the list of files via a file
👍 This is a common practice. Within build system, such files are called [“response” files](https://msdn.microsoft.com/en-us/library/3te4xt0y.aspx)
---
### Author:  [@fedorov](https://discourse.slicer.org/u/fedorov)
#### Post date: [May 30, 2017, 5:07pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/13 "2017-05-30T17:07:32Z")
@jcfr thanks for the response. Do you have any specific concerns with the proposed plan?
I think the issue of improving ITK is an important, but a separate one. I would prefer not to wait for the ITK improvements to be developed, tested and integrated to resolve this specific issue, unless there are really good reasons.
---
### Author:  [@lassoan](https://discourse.slicer.org/u/lassoan)
#### Post date: [May 30, 2017, 6:18pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/14 "2017-05-30T18:18:44Z")
I like the proposed solution of implementing this by making vtkITKArchetypeImageSeriesScalarReader smarter.
I have just a small comment: it would be a bit nicer to make the DICOM reader selector a string attribute (PreferredDICOMReader, PreferredDICOMIOMethod, PreferredIOClass, …), instead of a Boolean flag (UseGDCMImageIO), as in the future we will probably want to have more options (for example, different modes for using DCMTK or GDCM, auto-select between different readers, use other toolkits) without changing the interface.
It would be useful to have a way to load using DCMTK, even when GDCM returns something. For this, the DICOMScalarVolumePlugin could always offer two loadables for a series: a GDCM-based and a DCMTK-based. By default, if GDCM can interpret the image, the GDCM-based loadable would have higher confidence (so it would be selected by default).
---
### Author:  [@thewtex](https://discourse.slicer.org/u/thewtex)
#### Post date: [May 30, 2017, 6:21pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/15 "2017-05-30T18:21:01Z")
> [@jcfr](#):
>
> done before the ITK reader is instantiated
>
> May this could be added to ITK … in that case a list of readers could be returned. @thewtex Do you think that make sense or should Slicer team proceed as described ?
> [@](#):
>
> I think the issue of improving ITK is an important, but a separate one. I would prefer not to wait for the ITK improvements to be developed, tested and integrated to resolve this specific issue, unless there are really good reasons.
Yes, I think it is a idea to have the DICOM image type code available in ITK without duplication, but it should not be a blocker to move forward. Could an issue please be created here:
[GitHub - KitwareMedical/ITKDICOM: Better support for DICOM in ITK.](https://github.com/KitwareMedical/ITKDICOM)
that points to the appropriate code as a reference?
The overall plan sounds good 👍
Worth noting, @pieper did some digging the other week and found these DICOM datasets from GDCM:
[Grassroots DICOM / Gdcmdata / [a0bba5]](https://sourceforge.net/p/gdcm/gdcmdata/ci/master/tree/)
These are a good resource for regression testing.
---
### Author:  [@pieper](https://discourse.slicer.org/u/pieper)
#### Post date: [May 30, 2017, 8:55pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/16 "2017-05-30T20:55:57Z")
Thanks for the review and feedback everyone.
I agree with @lasson that we should rename the methods for selecting which toolkit to use. I also like the idea of offering the two methods as alternative loadables so the user can pick which to use in Advanced mode of the DICOM browser.
I’m also thinking it could make sense to write a test that checks out Matheiu’s repository (beefed up with any additional examples) and then runs both readers and compares the results. I wouldn’t want to make this a Nightly test, but it would make sense to be able to run it when the code changes.
---
### Author:  [@pieper](https://discourse.slicer.org/u/pieper)
#### Post date: [May 30, 2017, 9:12pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/17 "2017-05-30T21:12:46Z")
Since Andrey is traveling I went ahead and filed the issue on ITKDICOM:
[https://github.com/KitwareMedical/ITKDICOM/issues/7](https://github.com/KitwareMedical/ITKDICOM/issues/7)
---
### Author:  [@pieper](https://discourse.slicer.org/u/pieper)
#### Post date: [June 23, 2017, 12:35am UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/18 "2017-06-23T00:35:47Z")
I just merged the topic to address this.
>
>
> This issue was created automatically from an original Mantis Issue. Further discussion may take place here.
I believe it captures most (all?) of the agreed upon approaches discussed here.
TL;DR People should not notice anything different for now, since GDCM will still be the default reader, but if it fails to load correctly DCMTK will be used as a backup. There’s now a settings panel for DICOM where people can choose to use DCMTK if they want to experiment.
There is still this known issue outstanding,
>
>
> This issue was created automatically from an original Mantis Issue. Further discussion may take place here.
but report if you see other issues.
---
### Author:  [@ihnorton](https://discourse.slicer.org/u/ihnorton)
#### Post date: [June 23, 2017, 1:41pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/19 "2017-06-23T13:41:00Z")
Any deprecation schedule, to avoid the conversation in two years about “can’t remove GDCM **_OR_** DCMTK because it might break someone’s volume”? 😄
By the way, when I looked at the original issue that prompted this, I noticed that some slices of the sample data did in fact have thickness set to 0 – so GDCM wasn’t lying. I believe it worked in DCMTK because they don’t look at that tag at all (and then Slicer recalculates the spacing from IPP anyway).
---
### Author:  [@pieper](https://discourse.slicer.org/u/pieper)
#### Post date: [June 23, 2017, 3:42pm UTC](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354/20 "2017-06-23T15:42:55Z")
> [@ihnorton](#):
>
> Any deprecation schedule, to avoid the conversation in two years about “can’t remove GDCM OR DCMTK because it might break someone’s volume”? 😄
I think this is a long-term issue for the reasons I mentioned above - we don’t have a good way of knowing the impact of any particular change because we cannot test it with users’ actual data. For the self test I implemented code to load with both methods and compare the results. I was thinking to make that a selectable option so that we could start experimenting about when one fails but the other works.
Realistically what we need is a very large regression testing suite of data that somehow defines the data that we do support. This would allow changes to be made on at least some solid footing.
Unfortunately I don’t think there are any easy answers…
[Next page](https://discourse.slicer.org/t/slicer-dicom-scalar-volume-plugin-relies-on-old-gdcm-why-do-we-not-use-dcmtk/354.md?page=2)