Make DICOM QR queries inspectable

Issue #934 resolved
Ed McDonagh created an issue

Sometimes DICOM Q-R results are not easy to understand why particular studies/objects are retrieved and not others. Proposal is to enable the queries and their results to be stored so that they can be inspected.

Additionally, enable inspection of the success or errors associated with study imports.

Comments (38)

  1. Ed McDonagh reporter

    Sorry, deleted comment because I thought it was an issue with not having the venv activated. However, it is the same when it is (though this might still be the problem).

    When attempting to import an RDSR in this branch, installed on Windows, I get the following error:

    (venv) C:\Users\mcdonaghe\Documents\research\test788>C:\Users\mcdonaghe\Documents\research\test788\venv\Scripts\python.exe C:\Users\mcdonaghe\Documents\research\test788\venv\Scripts\openrem_rdsr.py C:\Users\mcdonaghe\Documents\research\test788\orthanc\dicom\7d920505-66582151-91b80675-3cc75542-104b5862.dcm
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\multiprocessing\spawn.py", line 116, in spawn_main
        exitcode = _main(fd, parent_sentinel)
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\multiprocessing\spawn.py", line 125, in _main
        prepare(preparation_data)
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\multiprocessing\spawn.py", line 236, in prepare
        _fixup_main_from_path(data['init_main_from_path'])
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\multiprocessing\spawn.py", line 287, in _fixup_main_from_path
        main_content = runpy.run_path(main_path,
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\runpy.py", line 263, in run_path
        return _run_module_code(code, init_globals, run_name,
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\runpy.py", line 96, in _run_module_code
        _run_code(code, mod_globals, init_globals,
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\runpy.py", line 86, in _run_code
        exec(code, run_globals)
      File "C:\Users\mcdonaghe\Documents\research\test788\bbOpenREM\openrem\scripts\openrem_rdsr.py", line 31, in <module>
        b = run_in_background_with_limits(
      File "c:\users\mcdonaghe\documents\research\test788\bbopenrem\openrem\remapp\tools\background.py", line 183, in run_in_background_with_limits
        p.start()
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\multiprocessing\process.py", line 121, in start
        self._popen = self._Popen(self)
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\multiprocessing\context.py", line 224, in _Popen
        return _default_context.get_context().Process._Popen(process_obj)
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\multiprocessing\context.py", line 326, in _Popen
        return Popen(process_obj)
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\multiprocessing\popen_spawn_win32.py", line 45, in __init__
        prep_data = spawn.get_preparation_data(process_obj._name)
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\multiprocessing\spawn.py", line 154, in get_preparation_data
        _check_not_importing_main()
      File "C:\Users\mcdonaghe\AppData\Local\Programs\Python\Python38\lib\multiprocessing\spawn.py", line 134, in _check_not_importing_main
        raise RuntimeError('''
    RuntimeError:
            An attempt has been made to start a new process before the
            current process has finished its bootstrapping phase.
    
            This probably means that you are not using fork to start your
            child processes and you have forgotten to use the proper idiom
            in the main module:
    
                if __name__ == '__main__':
                    freeze_support()
                    ...
    
            The "freeze_support()" line can be omitted if the program
            is not going to be frozen to produce an executable.
    Traceback (most recent call last):
      File "C:\Users\mcdonaghe\Documents\research\test788\venv\Scripts\openrem_rdsr.py", line 7, in <module>
        exec(compile(f.read(), __file__, 'exec'))
      File "C:\Users\mcdonaghe\Documents\research\test788\bbOpenREM\openrem\scripts\openrem_rdsr.py", line 37, in <module>
        wait_task(t)
      File "c:\users\mcdonaghe\documents\research\test788\bbopenrem\openrem\remapp\tools\background.py", line 238, in wait_task
        task.refresh_from_db()
    AttributeError: 'NoneType' object has no attribute 'refresh_from_db'
    

    Any ideas @Jannis Widmer ?

  2. David Platten

    Hi. I’ve just downloaded the latest version of this branch and tried to import one of the test CT RDSR files:

    (openrem-37) D:\code\python\openrem\openrem>python scripts\openrem_rdsr.py remapp\tests\test_files\CT-RDSR-Siemens-Multi-2.dcm

    When I run the above command an entry appears in the “tasks” page of OpenREM stating that the import failed:

    Traceback (most recent call last): File "d:\code\python\openrem\openrem\remapp\tools\background.py", line 128, in run_as_task func(*args, **kwargs) File "d:\code\python\openrem\openrem\remapp\extractors\rdsr.py", line 2479, in rdsr _rdsr2db(dataset) File "d:\code\python\openrem\openrem\remapp\extractors\rdsr.py", line 2187, in _rdsr2db record_task_related_query(study_uid) File "d:\code\python\openrem\openrem\remapp\tools\background.py", line 323, in record_task_related_query "started_at" File "c:\pythonVirtualEnvs\openrem-37\lib\site-packages\django\db\models\query.py", line 649, in latest return self.reverse()._earliest(*fields, field_name=field_name) File "c:\pythonVirtualEnvs\openrem-37\lib\site-packages\django\db\models\query.py", line 643, in _earliest return obj.get() File "c:\pythonVirtualEnvs\openrem-37\lib\site-packages\django\db\models\query.py", line 408, in get self.model._meta.object_name remapp.models.DicomQuery.DoesNotExist: DicomQuery matching query does not exist.12 May 2022, 8:33 a.m.Failure

  3. Ed McDonagh reporter

    Silly thing - you did do a database migration I assume? Can’t quite work out from that jumble what the error is 😆

    Which branch were you using? My new one (issue934FixScripts)? And was this the first RDSR you imported? Did it already exist?

    I’ve just tried importing that test file on my instance on Windows, and it worked fine…

  4. Ed McDonagh reporter

    That will definitely require a database migration. @Jannis Widmer is it in a state to test?

  5. David Platten

    I’m now looking at that branch. I have two failed CT imports which I am deleting. The “review” page has a button to click “delete entries and table” or words to that effect. When you click on it it is replaced with a red “Delete” and white “Cancel” button. It takes ages for these buttons to appear after you’ve clicked on the initial delete.

  6. David Platten

    I’m still getting the same error on the tasks page when I try to import:

    (openrem-37) D:\code\python\openrem\openrem>python scripts\openrem_rdsr.py remapp\tests\test_files\CT-RDSR-Siemens_Flash-TAP-SS.dcm

    I’m using the issue788ProcessBasedBackground branch and python 3.7 on Windows 10. I’ve done a “makemigrations remapp” and “migrate remapp”

  7. David Platten

    I’m running a direct install on Windows 10. Are there any additional python packages that I need to install?

  8. David Platten

    Just tried a DX import and that failed with the same sort of message in the tasks view. @Ed McDonagh - have you successfully imported a study using a native Windows OpenREM installation?

  9. Ed McDonagh reporter

    I just did a pip install -e . within the git folder.

    Yes, I have import everything fine. I’m currently struggling to get my test Orthanc instance to retrieve back to me, but from disk imports are good.

  10. Ed McDonagh reporter

    Now I’ve sorted out my Orthanc config (type in OpenREM instance hostname), the retrieve has resulted in an error on import for all studies. I’m taking a look now.

  11. Ed McDonagh reporter

    The error on importing the DX file is due to the anonymisation routine the PACS used when I exported some studies deleted the study date, which then falls over when we attempt to derive study_workload_chart_time.

    Less clear what is happening to the CT RDSRs, but it doesn’t look to be related to the new code. I’m going to export with a different anyonymisation setting and check.

  12. David Platten

    @Ed McDonagh it sounds like the errors you are seeing are different to the ones I have. Are you using Windows?

  13. Ed McDonagh reporter

    With dates, the DX images import fine. But the CT RDSRs are still causing errors:

    Traceback (most recent call last): 
    File "c:\users\mcdonaghe\documents\research\test788\bbopenrem\openrem\remapp\extractors\rdsr.py", line 1897, 
    in _generalequipmentmoduleattributes for content in dataset.ContentSequence 
    File "C:\Users\mcdonaghe\Documents\research\test788\venv\lib\site-packages\pydicom\dataset.py", line 778, 
    in __getattr__ return object.__getattribute__(self, name) AttributeError: 'FileDataset' object has no attribute 'ContentSequence' 
    
    During handling of the above exception, another exception occurred: 
    Traceback (most recent call last): 
    File "c:\users\mcdonaghe\documents\research\test788\bbopenrem\openrem\remapp\tools\background.py", line 128, 
    in run_as_task func(*args, **kwargs) 
    File "c:\users\mcdonaghe\documents\research\test788\bbopenrem\openrem\remapp\extractors\rdsr.py", line 2479, 
    in rdsr _rdsr2db(dataset) 
    File "c:\users\mcdonaghe\documents\research\test788\bbopenrem\openrem\remapp\extractors\rdsr.py", line 2291, 
    in _rdsr2db _generalequipmentmoduleattributes(dataset, g) 
    File "c:\users\mcdonaghe\documents\research\test788\bbopenrem\openrem\remapp\extractors\rdsr.py", line 1908, 
    in _generalequipmentmoduleattributes for content in dataset.ContentSequence 
    File "C:\Users\mcdonaghe\Documents\research\test788\venv\lib\site-packages\pydicom\dataset.py", line 778, 
    in __getattr__ return object.__getattribute__(self, name) 
    AttributeError: 'FileDataset' object has no attribute 'ContentSequence'
    

    (carriage returns are added in by me).

    Not sure what is going on yet - going to get some lunch!

  14. Ed McDonagh reporter

    Looks like for me that the stupid export anonymisation truncates the RSDR, thus rendering it useless. Exporting again…

  15. Ed McDonagh reporter

    Everything is working for me now I am not using corrupted data! Did you make any progress @David Platten ?

    I haven’t done extensive testing of course.

  16. Jannis Widmer

    @David Platten Just tried again, under windows 11 importing rdsr works for me (without Orthanc, just direct import). Anyway now the Merge-PR would be ready as well.

  17. Jannis Widmer

    Ok now also checked and updated with orthanc under windows. One problem seems to be that sqlite cannot handle a lot of concurrent request, which sometimes leads to crashing tasks. (It’s not an issue when using postgres)

  18. Ed McDonagh reporter

    We’ve got it working on David's setup with fresh databases, so it might be his development database is in a mess. We’re both going to try upgrading known good 0.10 databases a try again.

    What is “a lot” of concurrent requests? How hard did you have to work it?

  19. Ed McDonagh reporter

    I have now migrated a copy of a 0.10 production database and I am getting the same error as David did (packages\django\db\models\query.py", line 406, in get raise self.model.DoesNotExist( remapp.models.DicomQuery.DoesNotExist: DicomQuery matching query does not exist.)

    David’s solution was to delete the content of the following tables:

    • remapp_dicomqrrspstudy
    • remapp_dicomqrrspseries
    • remapp_dicomqrrspimage
    • remapp_dicomquery

    So it seems that the import task is is checking something in those tables for a from-disk import, and it doesn’t work when they have old data in…

  20. Ed McDonagh reporter

    So the issue is line 324 in background.py where started_at__isnull is filtered on from the DicomQuery table and as it is a new field that can be blank or null, it won’t exist on the migrated values. We could set a default for migration, we could try: except: or we could improve the function a little!

  21. Ed McDonagh reporter

    queries = DicomQuery.objects.filter(started_at__isnull=False) returns an empty queryset, with no error (the values are null).

    As soon as you do further filtering on that empty queryset, we get the error

    >>> q = DicomQuery.objects.filter(started_at__isnull=False).latest("started_at")
    Traceback (most recent call last):
      File "<console>", line 1, in <module>
      File "C:\test94and788\venv\lib\site-packages\django\db\models\query.py", line 649, in latest
        return self.reverse()._earliest(*fields, field_name=field_name)
      File "C:\test94and788\venv\lib\site-packages\django\db\models\query.py", line 643, in _earliest
        return obj.get()
      File "C:\test94and788\venv\lib\site-packages\django\db\models\query.py", line 406, in get
        raise self.model.DoesNotExist(
    remapp.models.DicomQuery.DoesNotExist: DicomQuery matching query does not exist.
    

  22. Jannis Widmer

    Regarding sqlite it seems a bit like there is no real way around it: https://stackoverflow.com/questions/31547234/django-sqlite-database-is-locked

    As soon as you have concurrent database writes you have a certain potential to run into problems. It’s worse since I do this somewhat dirty way of checking if a task can be executed via peridically looking at the db, but it would be an issue in any case. I somewhat ameliorated the problem by at least catching the ‘locked’ error when entering the tasks, but there can still be errors in the actual extractors if they execute in parallel. (Which also means it’s not really an issue of having a lot of parallism, that just increases the probablity of a problem)

    By the way a very simple fix for this would be to reverse what I’ve done to the Orthanc .lua - not execute extractors in parallel but sequentially. That has other problems, if an extractor takes longer to complete than the remote DICOM node keeps up the connection the move will fail on half the way (since the connection is not usable anymore).

  23. Log in to comment