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

MR Dicom writer fails #1128

Closed
johannesmayer opened this issue Jul 28, 2022 · 7 comments · Fixed by #1141
Closed

MR Dicom writer fails #1128

johannesmayer opened this issue Jul 28, 2022 · 7 comments · Fixed by #1141

Comments

@johannesmayer
Copy link
Member

johannesmayer commented Jul 28, 2022

The following error crashes the gadgetron server when trying to write an MR reconstruction to DICOM with the script below:

... some Gadgetron stream output...

07-28 14:18:04.857 INFO [GadgetStreamController.cpp:313]   Gadget class: DicomFinishGadget
07-28 14:18:04.857 INFO [GadgetStreamController.cpp:328]   Gadget parameters: 0
07-28 14:18:04.857 INFO [GadgetStreamController.cpp:356] Gadget Stream configured
terminate called after throwing an instance of 'std::runtime_error'
  what():  Unable to load ISMRMRD Meta XML document
Aborted (core dumped)

The script succeeds at writing dicom files for idx_file_to_run = 0,1 (where also the attribute XML file is generated) and fails for idx_file_to_run = 2,3

import sirf.Gadgetron as pMR
import numpy as np 
import os

def normalise_image_data(img):

    img = img.abs()

    img_max = np.max(img.as_array())
    img_min = np.min(img.as_array())

    img = 2**16 * (img - img_min)/(img_max-img_min)

    return img

sirf_install_path = str(os.getenv("SIRF_INSTALL_PATH"))
fpath_in = sirf_install_path + "/share/SIRF-3.3/data/examples/MR/" 

fnames_in = ["simulated_MR_2D_cartesian.h5",
"grappa2_1rep.h5",
"simulated_MR_2D_cartesian_Grappa2.h5",
"grappa2_6rep.h5"]

fname_out = "/media/sf_CCPPETMR/tmp_recon.dcm" # or whatever path you have write access to

idx_file_to_run = 0 # or 1, 2, 3
ad = pMR.AcquisitionData(fpath_in + fnames_in[idx_file_to_run])
ad = pMR.preprocess_acquisition_data(ad)
ad.sort()

recon = pMR.CartesianGRAPPAReconstructor()
recon.set_input(ad)

recon.process()
id = recon.get_output()
id = normalise_image_data(id)

id.write(fname_out)

I am on a VM at origin/master and build against ISMRMRD/v1.7.0 and Gadgetron is also built against this ISMRMRD version at the commit b6191eaaa72ccca6c6a5fe4c0fa3319694f512ab.

@evgueni-ovtchinnikov
Copy link
Contributor

@johannesmayer looks like another annoying Gadgetron's idiosyncrasy in handling image processing chains: it took me adding

    std::cout << attributes() << '\n';

in ImageWrap::real (any place) to be able to run all your tests successfully - please try the same and let me know the result.

BTW Gadgetron error messages , see gadgetron_dicom_write_failure.txt, suggest that Gadgetron starts 4 dicom image writing chains simultaneously instead of one!

@johannesmayer
Copy link
Member Author

Hmm that is strange, adding that line did not change anything for me, the tests fail the same.
Also I can't reproduce the protocol you sent, because it never activates any MRIImageReader Gadget.

@evgueni-ovtchinnikov
Copy link
Contributor

@johannesmayer I only sent you a very small portion of my Gadgetron output simply to show you that Gadgetron was running 4 image writing chains, which indicates multithreading problems.

@johannesmayer
Copy link
Member Author

johannesmayer commented Jul 28, 2022

If I run it with a gadgetron server from a docker container from dockerhub at gadgetron/ubuntu_1804_no_cuda instead of the one that I built on my VM it clears the above tests, but only when you add the line in ImageWrap::real() that you suggested.

Just calling attributes() without passing it to std::cout does not work.

@johannesmayer
Copy link
Member Author

I also tried to write some patient data reconstructions where it then fails also with the std::cout

@evgueni-ovtchinnikov
Copy link
Contributor

@johannesmayer Sending attributes to std::cout never meant to solve the problem - I did it just to check that attributes were as expected. Since this step miraculously helps in some cases and does not in others, I am certain we have multithreading problems, which is very bad news as they are extremely hard to sort out.

We already run into this kind of problems with three-chains demo I believe, and eventually I abandoned the faulty third chain, which was an image-processing chain, just the same kind as the one used by our dicom image writer.

I believe only Gadgetron experts could help us with this, but creating a Gadgetron issue would take a lot of effort I am afraid - currently, gadgetron_ismrmrd_client does not work with image processing chains, it expects acquisitions on input, so I would need to code a client that does, and without using any SIRF stuff.

Anyone heard about any hdf5-to-dicom converters perchance?

@KrisThielemans
Copy link
Member

See #430 for an attempt to update the threading code, but it's probably out-of-date.

evgueni-ovtchinnikov added a commit that referenced this issue Oct 26, 2022
* introduced a quick fix for dicom output failure #1128

* small corrections (missing/wrong dcm_prefix)

* git commit -m "optional dcm_prefix argument -> set_dcm_prefix() method"

Replaced the use of optional dcm_prefix argument in process() method
with the use of set_dcm_prefix() method in reconstructor classes

* added dcm output option to fully sampled single chain demo

* small dicom-related amendments in MR ImageReconstructor

* added brief descriptions of xml definition generators in gadget_lib.h [ci skip]

* small dicom-related amendments/corrections

* [ci skip] updated CHANGES.md

Co-authored-by: Kris Thielemans <[email protected]>
evgueni-ovtchinnikov added a commit that referenced this issue Nov 28, 2022
…ds Gadgetron 4.1.2 or later). (#1143)

* fixed dicom output via ImageProcessor chain; gadgetron 3.17 fails sometimes

* [ci skip] restored several-chains demo following PR 1143 bugfix

* [ci skip] removed obsolete two Gadgetron chains demo

* attended to Codacy issues

* [ci skip] removed commented-out DICOM quick-fix lines in grappa_basic.py

* attended to Codacy issues

* small amendments applied

* multi-chain demo to complain about old Gadgetron

* [ci skip] commented out quick fix throw in get_output()

* [ci skip] updated CHANGES.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants