Slicer custom application deployment to many computers

[spycolyf (Doug Porter)]

@spycolyf

Now trying to figure out how to insert the mandatory (0002, 0000) into the header, and that should fix that error.

Done, no more errors in the log files.

[spycolyf (Doug Porter)]

@spycolyf

Thanks, how about the Temporary database folders? They are not so temporary. They don’t seem to be ever deleted.

Do we need to manage those Temporary Database folders?

Thanks

[spycolyf (Doug Porter)]

Strange, I thought I fixed the logger problem because it worked earlier. But then I tried it on a large series I used on Friday, and got 1000s of lines indicating …

[CRITICAL][Stream] 01.08.2021 13:35:48 [] (unknown:0) - W: DcmMetaInfo: No Group Length available in Meta Information Header
[CRITICAL][Stream] 01.08.2021 13:35:48 [] (unknown:0) - W: DcmItem: Length of element (7fe0,0010) is not a multiple of 2 (VR=OW)
[CRITICAL][Stream] 01.08.2021 13:35:48 [] (unknown:0) - W: DcmItem: Non-standard VR '  ' (00\00) encountered while parsing element (0000,0000), assuming 2 byte length field
[CRITICAL][Stream] 01.08.2021 13:35:48 [] (unknown:0) - W: DcmItem: Dataset not in ascending tag order, at element (0000,0000)
[CRITICAL][Stream] 01.08.2021 13:35:48 [] (unknown:0) - W: DcmItem: Non-standard VR '  ' (00\00) encountered while parsing element (0000,0000), assuming 2 byte length field

… which continued for 1000s of lines. I then observed the DICOM headers. 7FE0 was set to OB, not OW as the logs indicated. And 0002, 0000 was indeed present contrary to what the logs indicated. I tried many times; no luck.

However, after I deleted all of the Temporary Databases and tried again, it worked. Therefore, the Temporary databases need to be deleted immediately after every launch. But I’m assuming this needs to be done very carefully as it may cause issues with multiple instances of the app.

Thanks

[lassoan (Andras Lasso)]

@spycolyf

I then observed the DICOM headers. 7FE0 was set to OB, not OW as the logs indicated. And 0002, 0000 was indeed present contrary to what the logs indicated. I tried many times; no luck.

DICOM files are not mutable. SOP instance UID of a DICOM files fully identifies the content of the file. Therefore, if you change a DICOM file you must always generate a new SOP instance UID (and usually series instance UID as well).

Maybe what happened is that when you patched the DICOM files you did not update the SOP instance UID and you still had the old files in the database. During DICOM import, Slicer detected that the files have been already imported (based on the SOP instance UID match) and therefore it did not import the files again.

@spycolyf

However, after I deleted all of the Temporary Databases and tried again, it worked.

Currently, TemporaryDatabase context manager just resets the database but does not delete the files. I’m not sure what the reason is, but probably it is not valid anymore, so we’ll just restore the original behavior of deleting the temporary files:

BUG: Remove temporary database files when the database is closed

[jcfr (Jean Christophe Fillion Robin (Kitware))]

@lassoan

TemporaryDatabase context manager just resets the database but does not delete the files. I’m not sure what the reason is, but probably it is not valid anymore, so we’ll just restore the original behavior of deleting the temporary files:

@spycolyf After you test the changes proposed by @lassoan locally, let us know. Then, we will integrate the changes in Slicer and also backport it into the branch used to build SlicerQReads. See here and here

[spycolyf (Doug Porter)]

@jcfr

After you test the changes proposed by @lassoan locally, let us know. Then, we will integrate the changes in Slicer and also backport it into the branch used to build SlicerQReads. See here and here

You guys are so great!

So to be clear there are 4 files to change:

Those files are here: https://github.com/Slicer/Slicer/pull/5764/files/9ac1329084af7e1e8250aad57ca6d670a4408aa6

• qMRMLCheckableNodeComboBox.cxx
• qMRMLPlotViewControllerWidget.cxx
• qSlicerSubjectHierarchySegmentationsPlugin.cxx
• DICOMUtils.py

I will make the changes my build (delete the red lines and insert the green lines) and rebuild. Is that a full rebuild with: “cmake --build . --config Release --target – /maxcpucount:8” ?

[lassoan (Andras Lasso)]

Merging this commit is sufficient:

BUG: Remove temporary database files when the database is closed

That’s a single file. It is a Python file, so you only need to “build” to copy the .py file from the source folder to the appropriate Python module folder.

[spycolyf (Doug Porter)]

@lassoan

That’s a single file. It is a Python file, so you only need to “build” to copy the .py file from the source folder to the appropriate Python module folder.

OK, I was confused by this [link] which shows the changes for “BUG: Remove temporary database files when the database is closed #5764.” I assumed I had to make all of those changes and I did. Should I undo the changes to the following files?

• qMRMLCheckableNodeComboBox.cxx
• qMRMLPlotViewControllerWidget.cxx
• qSlicerSubjectHierarchySegmentationsPlugin.cxx

[spycolyf (Doug Porter)]

@lassoan

That’s a single file. It is a Python file, so you only need to “build” to copy the .py file from the source folder to the appropriate Python module folder.

OK, I made the changes to DicomUtils.py. It’s the one located in my final executable package, “…\lib\SlicerQReads-4.13\qt-scripted-modules\DICOMLib” Folder… It doesn’t delete the temp DB files. Shouldn’t this work? Do I need to add a command to my QREADS.py file?

Here is the change I made to DicomUtils.py…

Changed FROM…

def deleteTemporaryDatabase(dicomDatabase, cleanup=True):
  """ Close temporary DICOM database and remove its directory if requested
  """
  dicomDatabase.closeDatabase()

  if cleanup:
    dicomDatabase.initializeDatabase()
    # TODO: The database files cannot be deleted even if the database is closed.
    #       Not critical, as it will be empty, so will not take measurable disk space.
    # import shutil
    # databaseDir = os.path.split(dicomDatabase.databaseFilename)[0]
    # shutil.rmtree(databaseDir)
    # if os.access(databaseDir, os.F_OK):
      # logging.error('Failed to delete DICOM database ' + databaseDir)

  return True

TO…

def deleteTemporaryDatabase(dicomDatabase, cleanup=True):
  """ Close temporary DICOM database and remove its directory if requested
  """
  dicomDatabase.closeDatabase()

  if cleanup:
    import shutil
    databaseDir = os.path.split(dicomDatabase.databaseFilename)[0]
    shutil.rmtree(databaseDir)
    if os.access(databaseDir, os.F_OK):
      logging.error('Failed to delete DICOM database ' + databaseDir)
      # Database is still in use, at least clear its content
      dicomDatabase.initializeDatabase()

  return True

The files are sill accumulating. What did I do wrong?

When this is done, I’ll be ready to deploy to 30,000 workstations. So, I need to do this somehow.

Thanks

[lassoan (Andras Lasso)]

I’ve tested the code and for me it deleted the temporary folder. You can add a break point and debug the code step by step, or add some log messages to get more information on what is happening exactly.

[spycolyf (Doug Porter)]

@lassoan

’ve tested the code and for me it deleted the temporary folder. You can add a break point and debug the code step by step, or add some log messages to get more information on what is happening exactly.

My problem might be that I did not add the code to the correct file. I added it to the DicomUtlis.py file in my built package. I think I should not have to recompile since it’s Python. That should work, correct?

Exactly how did you do it?

[lassoan (Andras Lasso)]

@spycolyf

I added it to the DicomUtlis.py file in my built package.

That should work. You can attach a Python debugger or at least add a few log messages so that you can verify what code is executed.

[jcfr (Jean Christophe Fillion Robin (Kitware))]

re: patching SlicerQReads

For convenience, here is a PR backporting the change. Consider reviewing.
See https://github.com/KitwareMedical/SlicerQReads/pull/106

I made the changes to DicomUtils.py. It’s the one located in my final executable package …

Patching a local build or install tree without updating the source code is really discouraged as changes could be lost anytime.

[spycolyf (Doug Porter)]

Just to be sure, are these changes [link] to the following files needed? If not, then no recompile necessary because were making changes only to DicomUtlis.py. Correct?

• qMRMLCheckableNodeComboBox.cxx
• qMRMLPlotViewControllerWidget.cxx
• qSlicerSubjectHierarchySegmentationsPlugin.cxx

Maybe you already have the changes to those files already.

[spycolyf (Doug Porter)]

We’re almost ready for primetime. The release is Aug 24th and we are currently in the testing phase. I demoed 3D MPR to the team and my unit head. The image load times are down, the size is down by 200MB, the module will be pre-installed on all 50K of the computers. and everything is running pretty smooth. The only concern expressed was the need for a progress bar while waiting for the images to load; especially for large series. The blank screen with the white background in the 3D view while images are loading leaves the user wondering and worrying if the app crashed. Our docs are stressed out enough

Are you sure there’s no solutions as of now?

Thanks

[spycolyf (Doug Porter)]

@jcfr

Patching a local build or install tree without updating the source code is really discouraged as changes could be lost anytime.

Thanks @jcfr. Yes I know. I do this only to test things out first and then I make it permanent by copying the changes to the source.

Thanks

[pieper (Steve Pieper (Isomics, Inc.))]

@spycolyf

The only concern expressed was the need for a progress bar while waiting for the images to load; especially for large series.

Were you able to have your version of the code open source? If so send a link to the place where you are loading the series and depending on the method we can probably give advice on how to add a progress bar.

[jamesobutler (James Butler)]

The Slicer custom app code is here:

GitHub - KitwareMedical/SlicerQReads: Slicer-based implementation of QREADS...

The custom version of Slicer used is here

GitHub - KitwareMedical/Slicer at SlicerQReads-v4.13.0-2021-04-26-d06e71c8a6

[spycolyf (Doug Porter)]

Hey guys, I fully intend to work on getting it in opensource and I know it’s to my benefit as well as the Slicer community to do so. If it were fully my decision to make, I would have already been in opensource. But I have to go through management and legal to get permission and they are not happy with my taking so long to get this out. I’m hopeful that I will be able to share it in opensource at the appropriate time.

Thanks

[pieper (Steve Pieper (Isomics, Inc.))]

Thanks Doug, I didn’t want to put you on the spot, just wanted to say it can be hard sometimes to guess what to suggest without being sure of how the code is working.

Can you describe which code path would benefit from a progress bar? Are you loading through the dicom module? The easiest could be to put up a static progress dialog that just says “Working…” and then hide it when the data is loaded. If you need an actual animated progress bar that accurately predicts how much work is left that can be trickier.