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

Structured console output #312

Closed
lassoan opened this issue Jul 21, 2019 · 4 comments
Closed

Structured console output #312

lassoan opened this issue Jul 21, 2019 · 4 comments

Comments

@lassoan
Copy link
Contributor

lassoan commented Jul 21, 2019

Currently, messages that are printed to the console are mostly optimized for human operators. It is not easy for applications that run dcm2niix to distinguish various message types.

Some warning messages are proceeded by the text "Warning: " and errors by "Error: ", and lines starting with tab character contain description of a loadable item and starting with a space can usually be ignored, but checking for these is not very robust.

There are several other messages that I would consider warnings but they are just logged as simple text ("Unsupported transfer syntax...", "Unable to determine...").

In addition to warning, error, and info, there are are also low-level debug ("dx=...", "instance=...") and non-essential ("Chris Rorden's dcm2niiX...", "Compression will be faster with pigz") messages that would be nice to distinguish.

It could be useful to have the chance of progress reporting (similar to how progress is reported in CLI modules used in many software, such as Slicer, MITK, Gimias, Girder plugins, and many other software - see https://www.slicer.org/wiki/Documentation/Nightly/Developers/SlicerExecutionModel#Showing_Progress_in_an_Application).

It could be nice to have a command-line switch, which would make the console output structured. Messages could be printed in XML or json format, with well-defined structure, which would be very easy to parse by any external software.

@neurolabusc
Copy link
Collaborator

These are great suggestions. I think they can be broadly broken into two categories:

  1. The messages generated by dcm2niix could be cleaned up a lot. The challenge is to do this in a way that does not break tools that assume the current conventions.
  • What is non-essential for one user (e.g. Chris Rorden's dcm2niiX version v1.0.20190410 (JP2:OpenJPEG) (JP-LS:CharLS) Clang8.1.0 (64-bit MacOS) might be used by another tool to ensure a specific version compiled with specific libraries (e.g JP2 and JP-LS support rare transfer syntaxes and dcm2niix compilation may or may not include support for these). Therefore, while I think a clean up of messages would help, the opportunity for unintended consequences is high.
  • Having feedback from users on the what constitutes a "message", "warning" and "error" would help. The latest commit includes your suggestion of promoting the unsupported transfer syntax to a warning. This example also shows the challenge: should this be a 'Warning' or an 'Error' - if one is converting hundreds of series that only include a few unsupported images a warning seems appropriate, but if converting a single image this feels like an error.
  • I think the warnings need extra help for CT images. My sense is that most dcm2niix users come from MR, and this is the modality used at my center. Messages like the "dx=..." will never be seen for typical MR, but common and probably confusing for CT. In the latest commit I have changed some of these such that the gory details are only provided if the software is run in Verbose mode.
  • Likewise, the latest commit will only show the hint "Compression will be faster with pigz" in verbose mode.
  • While the latest commit includes a few tiny changes, it seems that a new stable release is imminent. It seems like more fundamental changes should happen in the next cycle or in a separate branch to allow feedback from developers of different tools.
  1. I would encourage someone familiar with Slicer's extensions to generate a pull request to provide tighter integration (--xml, --logo, progress reporting, xml reporting). This might be a nice Project Week task - I would be happy to help, but do not think I could lead this. I think this could be done by placing a few strategic compiler directives such that user-specific attributes can be specified at compile time. A good template for this is the #ifdef USING_R directives that allow dcm2niix to be compiled into divest or the #ifdef myUseCOut that allows messages to be piped to a graphical QT interface.
  • I do think progress reporting would be generally useful. I will look at the Slicer documentation and see if I can come up with a general formula that can also be used for other graphical interfaces such as XCode.

neurolabusc added a commit that referenced this issue Jul 21, 2019
neurolabusc added a commit that referenced this issue Jul 22, 2019
@captainnova
Copy link
Collaborator

captainnova commented Jul 22, 2019 via email

@neurolabusc
Copy link
Collaborator

@lassoan the latest commit has a few changes

  • A basic audit of warnings, messages and errors.
  • New parameter --progress y which will report progress. Actual times depend a lot on disk speeds, whether the input files are compressed and whether the output files are compressed. There are three stages: (1) finding files, (2) reading DICOM headers, (3) reading DICOM images and writing NIfTIs. My software just assumes that stages (1) = 5%, (2) = 45%, (3) = 50%. The main idea is to give the user an idea that progress is happening. At the moment, the function printProgress() is called at every 5% increment. You can change what happens on printProgress() by creating a compiler ifdef in print.h - at the moment it just generates a text out (see below).
  • I also wrote a stub for a --xml command, one would need to modify the text At least one parameter.
  • This commit also attempts to clean up the code for detection of slice timing, is for faster converting of very large datasets, and fixes a bug for 3D volumes that exceed (2Gb). It passes all my validations, but it ended up being a very substantial commit. I would appreciate if people who handle very large datasets check out this commit (@captainnova).
$ dcm2niix --progress y -f %s_%p  /dsk/dcm
Chris Rorden's dcm2niiX version v1.0.20190720  (JP2:OpenJPEG) (JP-LS:CharLS) Clang10.0.0 (64-bit MacOS)
Progress: 0
Found 608 DICOM file(s)
Progress: 0.05
Progress: 0.1
Progress: 0.15
Progress: 0.2
Progress: 0.25
Progress: 0.3
Progress: 0.35
Progress: 0.4
Progress: 0.45
Progress: 0.5
Warning: Guessing slice times using ProtocolBlock SliceOrder=1 (seq=0, int=1)
Convert 150 DICOM as /dsk/dcm/6_Axial_EPI-FMRI_(Interleaved_S_to_I)a (64x64x15x10)
Progress: 0.6
Convert 2 DICOM as /dsk/dcm/5_EPI_PE=RLa (72x72x5x2)
Convert 150 DICOM as /dsk/dcm/4_Axial_EPI-FMRI_(Interleaved_I_to_S)a (64x64x15x10)
Progress: 0.75
Convert 2 DICOM as /dsk/dcm/4_EPI_PE=PAa (72x72x5x2)
Convert 150 DICOM as /dsk/dcm/7_Axial_EPI-FMRI_(Sequential_S_to_I)a (64x64x15x10)
Progress: 0.85
Convert 2 DICOM as /dsk/dcm/6_EPI_PE=LRa (72x72x5x2)
Convert 2 DICOM as /dsk/dcm/3_EPI_PE=APa (72x72x5x2)
Convert 150 DICOM as /dsk/dcm/5_Axial_EPI-FMRI_(Sequential_I_to_S)a (64x64x15x10)
Progress: 1

@neurolabusc
Copy link
Collaborator

Closing issue. Core functions included, Slicer team can submit pull request for Slicer-specific features.

yarikoptic added a commit to neurodebian/dcm2niix that referenced this issue May 6, 2020
* tag 'v1.0.20190720':
  Update dcm_qa submodule.
  Handle rare VRs (https://www.aliza-dicom-viewer.com/download/datasets)
  New default merge option "-m o" will merge CTs and segment MRIs (https://discourse.slicer.org/t/odd-dicom-import/7576)
  --progress option (rordenlab#314; rordenlab#312)
  Tune verbosity (rordenlab#312)
  GE Slice Timing From Protocol Data Block (rordenlab#311)
  Desparate guess of GE slice times from ProtocolDataBlock (clone dicm2nii, assumes TA close to TR)
  Support interleaved (rordenlab#309)
  Slice times from ucMode (rordenlab#309)
  GE number of slice discrepancy (rordenlab#306)
  Simplify slice timing code, develop stc validation (https://github.com/neurolabusc/dcm_qa_stc)
  hdr.vox_offset is a float not integer (https://www.nitrc.org/forum/message.php?msg_id=27155)
  BIDS tags for PASL fairest sequent
  Read 0019,1029 (rordenlab#296)
  Generate warning rather than error when palette colors are detected (UNFmontreal/Dcm2Bids#54)
  Update tinydir call (rordenlab#298)
  Bruker 4D datasets: correct slice ordering
  Bruker 4D datasets: Fix parametric maps grouping
  Changes up to divest v0.8.1
  Support isotropic scans for NRRD
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

3 participants