Make DICOM QR queries inspectable
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)
-
reporter -
reporter Minor changes to text. Might redraft when more familiar with the changes. Refs
#934[skip ci] docs only→ <<cset 8046fd093485>>
-
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 ?
-
Hmm I didn’t thouroughly test the import under windows, will have a look into it.
-
reporter Thanks
-
reporter Moved default_import.py to the extractors folder - see if it works when installed. Refs
#934→ <<cset 9cd0b6da1b2d>>
-
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
-
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…
-
OK. What branch should I try?
-
reporter A lot has happened since 8.30 this morning! @Jannis Widmer has been busy!
I’d say you’d want to look at issue94issue788issue934Merge now I think?
-
reporter That will definitely require a database migration. @Jannis Widmer is it in a state to test?
-
reporter Scrap that @David Platten - it isn’t ready. Use issue788ProcessBasedBackground instead
-
Yes I’m still doing the migrations and testing myself. Gonna create the PR on issue94issue788issue934Merge when I’m done.
Anyway issue788ProcessBasedBackground should be runnable.
-
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.
-
Are you running inside docker or with a direct install?
-
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”
-
I’m running a direct install on Windows 10. Are there any additional python packages that I need to install?
-
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?
-
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.
-
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.
-
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.
-
@Ed McDonagh it sounds like the errors you are seeing are different to the ones I have. Are you using Windows?
-
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!
-
reporter @David Platten yes, all in Windows
-
reporter Looks like for me that the stupid export anonymisation truncates the RSDR, thus rendering it useless. Exporting again…
-
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.
-
@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.
-
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)
-
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?
-
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…
-
reporter So the issue is line 324 in background.py where
started_at__isnull
is filtered on from theDicomQuery
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 couldtry: except:
or we could improve the function a little! -
reporter queries = DicomQuery.objects.filter(started_at__isnull=False)
returns an empty queryset, with no error (the values arenull
).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.
-
reporter Fixed in pull request #501
-
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).
-
reporter Attempt to trim a few of the +88 issues. Refs
#94,#788,#934.→ <<cset 1d35a052b783>>
-
reporter Merged in issue94issue788issue934tidying (pull request #504)
→ <<cset 460d4b1bb85d>>
-
reporter Updating changes with refs
#94,#788,#934, updating release notes. [skip ci] docs only→ <<cset 5aa12e5d8342>>
-
reporter - changed status to resolved
Merged in issue94issue788issue934Merge (pull request #500)
Issue94issue788issue934Merge
Fixes
#94,#788,#934, though there will be lots of docs revision to do.Approved-by: Ed McDonagh
→ <<cset ae758c3481e7>>
- Log in to comment
Spelling and spacing. Refs
#934[skip ci] no code change→ <<cset 1875c5b56424>>