QIICR / dcmqi

dcmqi (DICOM for Quantitative Imaging) is a free, open source C++ library for conversion between imaging research formats and the standard DICOM representation for image analysis results
https://qiicr.gitbook.io/dcmqi-guide/
BSD 3-Clause "New" or "Revised" License
237 stars 62 forks source link

E: PatientName (0010,0010) violates VR definition in PatientModule but actual value is correct #348

Closed mmromero closed 6 years ago

mmromero commented 6 years ago

Dear developers,

I am trying to create the structure report of a white matter hyper-intensity lesion segmentation with your docker container but no matter the Patient's Name I always get the same error (see below). The actual value is "N2D_PATIENT", but also tried with "xxx xxx" with the same result.

I can share the data with you, just let me know if you need it and I upload it.

Thank you very much. Miguel.

docker run -v /Users/miguel/Workspace/RAPIDS/SR/dcmqi_test:/tmp/dcmqi qiicr/dcmqi itkimage2segimage --inputDICOMDirectory /tmp/dcmqi/input_data --inputMetadata /tmp/dcmqi/wmh_segmentation.json --inputImageList /tmp/dcmqi/output_nifti/18991230_000000nifti2dicom044s001a001.nii.gz --outputDICOM /tmp/dcmqi/wmh_segmentation.dcm
dcmqi repository URL: git@github.com:QIICR/dcmqi.git revision: 72869c3 tag: latest-2-g72869c3
Searching recursively /tmp/dcmqi/input_data for DICOM files
Input image size: [240, 240, 48]
W: PatientName (0010,0010) violates VR definition in PatientModule
W: PatientSex (0010,0040) violates VR definition in PatientModule
W: AccessionNumber (0008,0050) violates VR definition in GeneralStudyModule
W: InstitutionName (0008,0080) violates VR definition in GeneralEquipmentModule
W: PatientAge (0010,1010) violates VR definition in PatientStudyModule
W: PositionReferenceIndicator (0020,1040) absent in FrameOfReferenceModule (type 2)
Directions: 0.997303 -0.0502721 -0.0534678
-0.0229997 -0.905936 0.422789
0.0696929 0.42042 0.904649

Processing input label Image (0x1889550)
  RTTI typeinfo:   itk::Image<short, 3u>
  Reference Count: 3
  Modified Time: 184
  Debug: Off
  Object Name: 
  Observers: 
    none
  Source: (none)
  Source output name: (none)
  Release Data: Off
  Data Released: False
  Global Release Data: Off
  PipelineMTime: 48
  UpdateMTime: 183
  RealTimeStamp: 0 seconds 
  LargestPossibleRegion: 
    Dimension: 3
    Index: [0, 0, 0]
    Size: [240, 240, 48]
  BufferedRegion: 
    Dimension: 3
    Index: [0, 0, 0]
    Size: [240, 240, 48]
  RequestedRegion: 
    Dimension: 3
    Index: [0, 0, 0]
    Size: [240, 240, 48]
  Spacing: [0.958333, 0.958333, 3]
  Origin: [-107.096, 80.3152, -82.9768]
  Direction: 
0.997303 -0.0502721 -0.0534678
-0.0229997 -0.905936 0.422789
0.0696929 0.42042 0.904649

  IndexToPointMatrix: 
0.955749 -0.0481774 -0.160403
-0.0220414 -0.868189 1.26837
0.066789 0.402902 2.71395

  PointToIndexMatrix: 
1.04066 -0.0239997 0.072723
-0.0524578 -0.945325 0.438699
-0.0178226 0.14093 0.30155

  Inverse Direction: 
0.997303 -0.0229997 0.0696929
-0.0502721 -0.905936 0.42042
-0.0534678 0.422789 0.904649

  PixelContainer: 
    ImportImageContainer (0x1884ec0)
      RTTI typeinfo:   itk::ImportImageContainer<unsigned long, short>
      Reference Count: 1
      Modified Time: 180
      Debug: Off
      Object Name: 
      Observers: 
        none
      Pointer: 0x7f569bb8a010
      Container manages memory: true
      Size: 2764800
      Capacity: 2764800

Found 2 label(s)
Skipping label 0
Processing label 1
Total non-empty slices that will be encoded in SEG for label 1 is 48
 (inclusive from 0 to 48)
E: PatientName (0010,0010) violates VR definition in PatientModule
FATAL ERROR: Writing of the SEG dataset failed! Please report the problem to the developers, ideally accompanied by a de-identified dataset allowing to reproduce the problem!
pieper commented 6 years ago

Hi @mmromero -

Thanks for the report šŸ‘

I'd try encoding the PatientName as a PN (person name) VR according to the standard:

ftp://dicom.nema.org/MEDICAL/Dicom/current/output/chtml/part05/sect_6.2.html

If it's still not working, can you post your wmh_segmentation.json file?

-Steve

mmromero commented 6 years ago

Dear Steve,

Thank you for the quick answer, I checked the VR PN format but I didn't see how a name of one word without special characters couldn't match it. I will check again, just to be sure.

I am running a test of dcmqi, for that I used the following wmh_segmentation.json:

{
  "ContentCreatorName": "Miguel",
  "ClinicalTrialSeriesID": "Session1",
  "ClinicalTrialTimePointID": "1",
  "SeriesDescription": "WMH Segmentation",
  "SeriesNumber": "300",
  "InstanceNumber": "1",
  "BodyPartExamined": "Brain",
  "segmentAttributes": [
    [
      {
        "labelID": 1,
        "SegmentDescription": "WMH",
        "SegmentAlgorithmType": "AUTOMATIC",
        "SegmentAlgorithmName": "IBBM Algorithm",
        "AnatomicRegionSequence": {
          "CodeValue": "T-A0100",
          "CodingSchemeDesignator": "SRT",
          "CodeMeaning": "Brain"
        },
        "SegmentedPropertyCategoryCodeSequence": {
          "CodeValue": "T-D0050",
          "CodingSchemeDesignator": "SRT",
          "CodeMeaning": "Tissue"
        },
        "recommendedDisplayRGBValue": [
          237,
          5,
          5
        ],
        "SegmentedPropertyTypeCodeSequence": {
          "CodeValue": "T-D0050",
          "CodingSchemeDesignator": "SRT",
          "CodeMeaning": "Tissue"
        }
      }
    ]
  ],
  "ContentLabel": "SEGMENTATION",
  "ContentDescription": "Image segmentation",
  "ClinicalTrialCoordinatingCenterName": "dcmqi"
}

Thank you, Miguel.

pieper commented 6 years ago

First glance that looks okay - I guess if you post the full dataset to dropbox or similar we could have a closer look (assuming no identifying patient info of course).

mmromero commented 6 years ago

Sure, you can download it from here: https://gigamove.rz.rwth-aachen.de/d/id/KfEouv7ymsysW9

fedorov commented 6 years ago

dciodvfy reports a number of errors for that dataset, I wonder if this causes the conversion problem too. Looks like you created that series with ITK, is this correct? If yes, what version did you use?

image

mmromero commented 6 years ago

Hi @fedorov. I created the dicoms from a nifti file using the nifti2dicom tool as follows:

nifti2dicom_macos_0.4.3-0r1 -i INPUT -o OUTPUT --studyid rapidstest --studyinstanceuid 1.2.826.0.1.3680043.2.1143.9925161747653993333667655489998915347 --patientage 33 --patientsex M --patientweight 85 --patientdob 19841101 --studydate 20180516 --seriesnumber 1 -a 1

The ITK version is 4.8.2.

If you think this is the problem I will populate all the fields with my values instead of using the default ones. The problem is that I only have the niftis for these data and I need to use the dicoms.

Thank you very much.

fedorov commented 6 years ago

Interesting. I have not seen that tool before, and indeed it is a good question - what tool we could recommend to create a DICOM image series from non-DICOM image ... I don't have a ready answer!

I am not sure it will help if you just not use the default values. It looks like it might be writing trailing characters that are not consistent with the standard, and I do not know how to confirm or fix this. I would need to dig into the standard more to see if I can figure it out. But if the issue is in your converter, I can't think of an alternative to suggest! .. :-(

pieper commented 6 years ago

Of course you can use Slicer to convert your files to dicom - either with a command line module (that you could script) or with the GUI.

https://www.slicer.org/wiki/Documentation/Nightly/Modules/CreateDICOMSeries

https://www.slicer.org/wiki/Documentation/Nightly/Modules/DICOM#DICOM_export

On Tue, Aug 7, 2018 at 12:16 PM, Andrey Fedorov notifications@github.com wrote:

Interesting. I have not seen that tool before, and indeed it is a good question - what tool we could recommend to create DICOM from non-DICOM ... I don't have a ready answer!

I am not sure it will help if you just not use the default values. It looks like it might be writing trailing characters that are not consistent with the standard, and I do not know how to confirm or fix this. I would need to dig into the standard more to see if I can figure it out. But if the issue is in your converter, I can't think of an alternative to suggest! .. :-(

ā€” You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/QIICR/dcmqi/issues/348#issuecomment-411114675, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHsfUlhDpqvyfHDsErRSG6xq8o2ABVAks5uOb1vgaJpZM4VyHiq .

fedorov commented 6 years ago

you can use Slicer to convert your files to dicom

... but that will mean the output will always be CT, and you cannot import metadata from another DICOM object as the tool you mentioned advertises it can do.

pieper commented 6 years ago

Actually I think if you imported the other data and then put your volume in the subject hiearchy node with the imported data then the exported data dicom files would inherit the study information.

The point about the exported dicom having a CT modality is valid - that's been a 'feature' of the CreateDICOMSeries module since the beginning but if that's the only objection and there's no other good tool, then making the modality into a command line argument would be a simple fix.

Perhaps @cpinter or @lassoan have other suggestions?

fedorov commented 6 years ago

making the modality into a command line argument would be a simple fix

I don't think it would be that simple. The tool would need to know what attributes are required by individual IODs, and be able to import those from the input reference instances. I think implementing that in a meaningful way will require a non-trivial effort.

Do we know of any other tool that can take a non-DICOM image and make a DICOM out of it?

pieper commented 6 years ago

That's my point - of course it's complicated to make everything perfect, but if all you need to do is help people in @mmromero's situation then there may not be any better tool. Plus improving the DICOM export from Slicer, even if imperfect, might still be useful.

Of course the other way to do it is to just write a program that generates the DICOM you wanted - e.g. with dcmtk, pydicom, or other method.

fedorov commented 6 years ago

Yes, I completely agree. I am just wondering if there is another ready-to-go tool that could help here.

pieper commented 6 years ago

Depending on how much data there is to convert and how variable it is, one brute force method is to use dcmdump and dump2dcm along with a script that slices up the volumes and does string replaces on the dump file. Not elegant but at least you know that if you start with a valid file of the type you are trying to create you will have the right dicom tags. This is not an elegant approach but it may be easier than learning how to do the same thing in dcmtk.

lassoan commented 6 years ago

You can specify any modality when you export an image from Slicer. In the DICOM export dialog, change Modality tag in Edit DICOM tags section.

In Data module, create subject and study, drag-and-drop the series (volume node) under the study, and right-click on the series to export it:

image

The DICOM export dialog is opened where several fields can be modified and then exported to DICOM file:

image

You can use dcmodify to change individual tags in a DICOM file, but it is not very convenient, because it is a command-line tool and you also need to write a short script that iterates through all files of a series to change modality of a volume.

mmromero commented 6 years ago

Thank you @lassoan, exporting dicoms from Slicer works. I managed to generate the segmentation report. Also @pieper and @fedorov, thank you for quick answer and support.

pieper commented 6 years ago

Thanks @mmromero for reporting and following up!

fedorov commented 6 years ago

@lassoan thank you for the details!

@mmromero glad the issue is resolved for you, and thank you for pointing us to the other tool you used. I was not aware of it.

If/when I figure out what was the issue with the output it produces, I will see if I can make a PR to fix the issue, since that tool looks more comprehensive than Slicer in customizing the output, and can be used in batch mode, which makes it unique - I don't know of something comparable. Meanwhile, I took note of the issue in https://github.com/biolab-unige/nifti2dicom/issues/33.

lassoan commented 6 years ago

You can use all Slicer features in batch mode, without GUI. If you ever need help in figuring out how to access a particular feature then ask on the Slicer forum.

"nifti2dicom" has a very narrow scope - it can only export a single 3D image volume. DICOM storage is much more than this. You may need to export segmentation objects, RT structure sets, registration objects, structured reports, complete studies with references between series, etc. Slicer can all do all this, using an extensible, plugin-based DICOM export infrastructure. It's far from perfect, but it's being continuously improved.

pieper commented 6 years ago

Andrey and I did this script to go from dicom to nrrd - it could definitely be generalize. Maybe we want to add some example going to dicom for various common types.

https://github.com/QIICR/dcmheat/blob/master/docker/SlicerConvert.py

fedorov commented 6 years ago

Andras, I agree with you, but scripting Slicer functionality in python, and running the result in batch mode has a very big startup overhead, and is associated with other inconveniences, such as tricks that need to be done for headless mode, large package size, platform-specific launcher issues. It might be a lot more convenient for users to use command line tools that are designed for the specific task (when they are available!) instead of learning how to write Slicer scripts, and then figuring other issues.

I am all for Slicer, as you know, but for example using Slicer for batch conversion of DICOM from command line is extremely slow and not convenient, so for that task I would rather use plastimatch, dcm2niix, or dicom2nifti, which are all maintained, and do the job well and much more efficiently.

In this particular case, indeed, if nifti2dicom is not supported (there has not been much activity, and I am not sure about the quality of the code), Slicer might be the only option.

lassoan commented 6 years ago

I believe that having so many simple, rigid, single-purpose dicomToX tools is bad, because these tools hide issues (by making random assumptions and ignore various potential problems) and cause fragmentation in the research community. It would be better to establish one common DICOM import/export tool framework that many people can use, customize, and extend.

I agree with your list of limitations above (although maybe the recently completed PythonSlicer interpreter solved some of these issues). This common DICOM converter framework does not have to be Slicer-based. However, it should be general, customizable, and extensible, similarly to Slicer's DICOM import/export infrastructure.

pieper commented 6 years ago

It would be great to factor out the code in slicer so that it could be used in other scenarios in addition to the slicer gui. We already do this for things like the diffusion plugin that uses DWIConvert under the hood or QuantitativeReporting that uses dcmqi.

Regarding your other points Andrey, while they are generally true I don't see things like package size or startup time as limiting factors. Converting files like this is not a real time activity and if you have a large number of dicom files to convert slicer is the least of your disk space concerns. No reason to prematurely optimize something like this.

jcfr commented 6 years ago

, and running the result in batch mode has a very big startup overhead,

We need to address this. (the first step was to lazily load CLIs which was implemented before 4.8.1, now when a specific module is request for batch processing, we need to improve the dependency management and load only the needed dependencies)

and is associated with other inconveniences, such as tricks that need to be done for headless mode,

That is currently a problem indeed.

Now, I think that putting effort into having a well maintained docker image supporting this (we could even have one based of "nvidia-docker") would address 80% of the batch processing use case.

large package size

Removing EMSegment, BRAINSTools and SimpleITK for the core should help.

platform-specific launcher issues.

Do you have more details ? (if that helps, the PythonSlicer launcher can now be used on all platform including installed macOS)

pieper commented 6 years ago

Now, I think that putting effort into having a well maintained docker image supporting this (we could even have one based of "nvidia-docker") would address 80% of the batch processing use case.

If it gets the job done I'm okay with a docker based solution, but of course that's the opposite of lightweight.

I would lean toward trying to make sure all the dicom conversion code is logic-only and could be imported without pulling vtkRendering or Qt into the python interpreter (PythonSlicer for now, ultimately a regular python).

Also, thinking about the actual workflow of converting a large set of nii files to dicom, I'd personally want to do a lot of QC checking because so many things could be inconsistent in the nifti files. So what I'd probably do is start with the CaseIterator and create a custom scripted module to call the dicom export into an organized directory hierarchy (and also into the Slicer dicom database).

fedorov commented 6 years ago

Great discussion, would be even better if we could have it in-person!

@lassoan

having so many simple, rigid, single-purpose dicomToX tools is bad, because these tools hide issues (by making random assumptions and ignore various potential problems) and cause fragmentation in the research community. It would be better to establish one common DICOM import/export tool framework that many people can use, customize, and extend.

First, I disagree that a general statement like this is accurate. Any tool has issues, and (at least for an outsider) makes random assumptions, and Slicer is no exception, but many of the converters have been around for quite a long time, and are quite robust (arguably, more robust than Slicer!) in dealing with various problems. I think non-Slicer converters from DICOM have larger user base, and may be more reliable just because more people tried and broke them before (e.g., most likely dcm2niix has a much larger user base than Slicer DWConverter).

Second, fragmentation is inevitable, since research community by definition has different interests and focus areas. But that is good! I do not believe in monolithic tools that solve all problems for everyone. Would it be better to establish one common tool? Maybe. But I don't see how it can be done practically. I wonder if there is any example where something like this has been done successfully in the research community.

@jcfr

platform-specific launcher issues. Do you have more details ? (if that helps, the PythonSlicer launcher can now be used on all platform including installed macOS)

The lack of console output on Windows is a big one for me, and based on many discussions over the past years.

@pieper

Regarding your other points Andrey, while they are generally true I don't see things like package size or startup time as limiting factors. Converting files like this is not a real time activity and if you have a large number of dicom files to convert slicer is the least of your disk space concerns. No reason to prematurely optimize something like this.

I think package size is an issue for someone composing a docker image. I think it is common for folks to try to minimize that, and this converter (~1Gb for Slicer?) would be just a small piece of it. For me, bottom line is using Slicer for converting DICOM in batch mode is cumbersome, but more importantly - we don't know how robust it is compared to other conversion tools (I am talking here about "from DICOM" pathway). What we do know is that the DICOM IO components of ITK need more love, we also know that for sure "assumptions were made" by people who are no longer around to maintain that code, and we also know that we do not have a robust regression testing for DICOM conversion in Slicer. So all things considered, I am leaning much more towards recommending the converters listed in the project I started last January. Unfortunately, it's been difficult to make time to continue with that project. If and when we establish some technical capability to compare converters across the board on a versatile dataset, that might be the first step towards educated decision making and harmonization of tools.

lassoan commented 6 years ago

Any tool has issues, and (at least for an outsider) makes random assumptions

I agree. My point is that based sharing solutions to common problems lowers the development and maintenance workload in the long term (more time is saved than the overhead of coordination between different groups). If you just let everybody do what feels convenient, the easy 90% of the code is reimplemented many times, and the difficult 10% has partial implementation in random toolkits. Eventually, a few well-designed, full-featured solutions emerge as winners, but this process can be accelerated with some conscious effort.

I do not believe in monolithic tools that solve all problems for everyone

I agree, monolithic tools are not well suited for this. The problem exactly is that various groups maintain their own monolithic tools, with zero modularity and potential for reuse or customization. Instead, we should have a small common framework that many groups are using and contributing to - with shared infrastructure components (such as tilted gantry, variable image slice spacing, patient name, etc. management) and application-specific plugins (that can handle import/export of a particular DICOM IOD).

The lack of console output on Windows is a big one for me

We've fixed CTK to have a full-featured interaction Python console, even on Windows. It works really well! However, these developments have not been merged yet . @jcfr could you please merge the pull requests and update Slicer master?

lassoan commented 6 years ago

By the way, this is a great discussion, which will be difficult to find later. Should we migrate this discussion to Slicer forum or somewhere else with better visibility?

fedorov commented 6 years ago

@lassoan absolutely, although I am not sure what is the good way to migrate. Did you mean to copy individual posts, or make a summary of the discussion?

fedorov commented 6 years ago

As I discovered while working on something else, plastimatch convert has a function to create DICOM series from an ITK volume. Looks like it only outputs CT, but it provides options to populate metadata from an existing DICOM dataset.

jcfr commented 6 years ago

I am not sure what is the good way to migrate

Look like this may be what we are looking for:

https://meta.discourse.org/t/introducing-github-issues-to-discourse/46671

jcfr commented 6 years ago

I re-opened the issue to see if I could select it using their system and it look like it will work. If there is consensus, I can proceed to the copy of the issue discussions into the support category on the Slicer forum.

After the import, we can split/delete/re-organize posts. Should we keep the same title of find a better one ?

image

lassoan commented 6 years ago

Yes, let's try this to see how well it works.

jcfr commented 6 years ago

This issue was copied to https://discourse.slicer.org/t/e-patientname-0010-0010-violates-vr-definition-in-patientmodule-but-actual-value-is-correct/3737

fedorov commented 6 years ago

@jcfr thank you!

Could you also add a link to the top message in the discourse discussion pointing back to this github issue as a source of the conversation? It is in fact somewhat strange that such link is not included by default while copying the conversation.

fedorov commented 6 years ago

@jcfr never mind - I was able to edit your message on discourse (!) to add that link myself!