I had no trouble loading this data on my local build (r22855).

Can anyone else confirm the error using this dataset?

I am unable to consistently reproduce this issue. It happens sometimes, but I was not able to identify the specific sequence of steps that would reliably lead to this issue.

Interesting - can you clarify where exactly the issue comes up? It looks to me like all the parsing and interpretation works fine, so maybe there's something else that wedges the DICOM browser code. Does the issue happen on import, when you select a series, when you examine or when you load?

Steve, I am able to consistently reproduce the first issue (only single slice showing up) with the today's nightly build and the dataset I shared with you earlier. Can you try that? Maybe the issue is specific to the nightly build or release build?

I cannot reproduce it using a recent nightly build on Windows. It's a very old issue, most probably the root cause have been fixed in the last 3 years.

There is definitely something weird going on with this data but it's hard to pin down.

I am able to reproduce some of this with today's nightly version on mac, but not in a debug build.

I could also reproduce odd behavior on windows using the current nightly (I don't have a debug build at the moment).

I did not observe the DICOM module losing content after loading.

What I do see:

Mac release (not debug):
Test1: fresh start of Slicer: use File->Add Data->Add File and pick 7.dcm => only one slice loads
Test2: fresh start of Slicer: use File->Add Data->Add File and pick 0.dcm => full volume loads
Test3: perform Test2, then load using any file in series => full volume loads
Test4: once volume loads once, it seem never to load again as a single slice using any method.

Windows release:
Test1: fresh start or not: use File->Add Data->Add File and pick any file => only one slice loads (whether or not Single File flag is picked)
Test2: fresh start or not: use DICOM module => full volume loads

These tests were done with the nightly builds marked 2017-06-11.

@lassoan can you confirm what I see on windows?

The DICOM file is certainly invalid (as decimal strings are not allowed to contain spaces).

The problem is that in vtkITKArchetypeImageSeriesReader::AnalyzeDicomHeaders(), image position is parsed like this:

if (sscanf( tagValue.c_str(), "%f\\%f\\%f\\%f\\%f\\%f", a, a+1, a+2, a+3, a+4, a+5 ) == 6)

This of course fails when there are extra spaces. The first value is filled, the rest is random memory junk (except in debug mode - in that case values may be initialized to a specific value).

Potential solutions:
A. Make parsing more robust (ignore spaces)
B. Make sure that such invalid files cannot be loaded (log a meaningful error message) and maybe add a rule to the DICOM patcher.

Which one would you prefer?

Thanks for debugging!

David Clunie usually promotes the "read loosely, write strictly" rule, so I think if something can be handled we should probably try.

I hate to make the user do extra steps when their source of data is bad, so making them run the patcher seems not optimal here.

In this case, the workaround is easy to implement, so let's go with reading loosely.

I don't think "reading loosely" is a good general principle, as 1. lots of workarounds make the code difficult to maintain and understand; 2. results of error-compensating mechanisms are unpredictable (if there are multiple loose interpretations that would fit a certain data, which one do you choose?). In many standards, loose interpretation of data is strictly forbidden.

Fixed in r26087

I don't think "reading loosely" is a good general principle

It is not so much a discussion about whether "reading loosely" is a good general principle. It is a matter of reality that if you don't use this principle in regards to reading DICOM images, the result will be a world with very little data, and a lot of unhappy users who don't want to know any of these details, and just want to see the image loaded.

I should also add that in this particular situation, it is not a random dataset, but a dataset coming from TCIA. TCIA is expected to be the place for "curated DICOM data", that creates certain expectation. It is indeed unfortunate that "curation" does not imply basic checking of the data, and does not implement some basic cleanup to fix issues like this one.

I was rushed when I typed the message earlier, so I didn't pull up the reference - have a look for Postel's Principle in these slides:

http://www.dclunie.com/papers/DICOMWeb_2015_Clunie_BestPracticesAndTools.pdf

I do like the idea of being "definite" on what we accept, to the extent that is possible.

Quoting David from the slides:

"The entire Study content is important
l Showing only a subset may be unsafe"

Isn't this ironic, considering that you don't have any standard-defined mechanism to confirm that your study (or even multi-instance series!) is complete ...

I find it just sad that such an important and in many aspects very well written standard is implemented so inconsistently. I don't know how this mess could be fixed, but providing official reference implementations and example data sets would solve many problems.

providing official reference implementations and example data sets would solve many problems

"You Can Lead a Horse to Water But You Can't Make It Drink"
http://markewbie.weebly.com/proverbs.html

Until solving those problems leads to higher profits by the manufacturers, or is mandated by the government as a regulation, this assumes good will and won't happen. I am not saying we should not try, quite the opposite, that is all we can do. But I am not that optimistic it would solve the problems of inconsistency and fragmentation any time soon.

Agreed!

In working on the dcmtk reader support the vtkITK code I've been really torn on how best to implement the testing. I feel like I really need terabytes of ground truth data that just doesn't exist, at least not in any remotely organized form.