MagneticResonanceImaging / MRIReco.jl

Julia Package for MRI Reconstruction
https://magneticresonanceimaging.github.io/MRIReco.jl/latest/
Other
85 stars 22 forks source link

Strip of IO Part in Dedicated Package #66

Closed tknopp closed 2 years ago

tknopp commented 2 years ago

After seeing https://github.com/JeffFessler/MIRTio.jl I am thinking about stripping of some IO parts from MRIReco.jl into a dedicated package. This has no high priority for me right now but I feel this could be one first steps towards package consolidation (aka the big JuliaImageRecon plan). Of course this move only makes sense if we merge MIRTio.jl and the IO parts of MRIReco.jl.

In the following I am using the term Files instead of IO since we have already a successful package called MPIFiles.jl that would nicely fit into the picture. But let us not focus on naming discussions before structural discussions.

There are following design question that I have:

  1. I would prefer going into the direction of having a familiy of packages MRIFiles.jl, CTFiles.jl, MPIFiles.jl since in my perspective there is too much domain specific stuff that would make a big package too complicated.
  2. If there is overlap in the vendor format one can create some packages that can be used in the above ones. For instance BrukerFile is used in both MPIFiles.jl and MRIReco.jl and I maintain two copies of the same code... So if the same holds for GE/Siemens/Philips, we would want that as well.
  3. Now concrete to MRIFiles.jl (working title!): In MRIReco.jl we made one important design decision: We use the ISMRMRD in-memory representation as the basis of an abstract datatype for all MR data formats (currently BrukerFile is the only alternative implementation). Our rational here is that the people behind ISMRMRD already have done this abstraction and therefore I trust that any MRI file can be converted into the ISMRMRD structs. Following that line of thoughts, we would try taking https://github.com/JeffFessler/MIRTio.jl/tree/main/src/ge-mri and filling the ISMRMRD structs instead of giving direct access as MIRTio.jl currently does. In other words: RawAcquisitionData would be the base interface for all vendor file formats.

This is up to discussion. Very important is of course the thought of @JeffFessler but I also would like to hear what @JakobAsslaender and also @mrikasper think. Would also be interesting to get some feeling what vendors we would actually target (or legally can). My focus on BrukerFile is certainly pretty exotic but it is just because I work with MRI/MPI devices from Bruker in my lab. I know that we should actually target the big vendors but I have neither data, nor specifications. So that would require community effort.

aTrotier commented 2 years ago

Hi,

You want to build a reader for each main vendors rather than using the ISMRMRD generated by e.g. siemens_to_ismrmrd ?

I am not working a lot on the .dat files (siemens rawdata files) because I am directly dumping the data from the scanner to my server in ISMRMRD but during sequence development I remember digging into the files sometimes to see why the converter is not working as expected. For the siemens conversion/reader there is already some stuff available in matlab and python.

MRIReco is one of the only toolbox that are available for Bruker files and due to the lack of metadata header and the possible divergence between the C part, having an easy access, modifications to your function enable to create a converter that works for each custom sequence (MP2RAGE example)

JeffFessler commented 2 years ago

I like the idea of gathering raw-data readers used for image reconstruction in one discoverable place and isolating them from recon packages. I used to have AVS .fld format readers in MIRT.jl but moved it to FileIO.jl instead (https://github.com/JuliaIO/AVSfldIO.jl) but that is a much simpler format and more generic than sinogram or k-space data. I also like the choice of using ISMRMRD as the standard for in-memory, even though I've never used it myself. A single Module for all MRI readers seems hard to maintain, but a single repo with multiple modules in it (like you've done so nicely in NFFT.jl) seems feasible. You have write access to JuliaImageRecon so feel free to make a repo whenever you want to get started. On my end this will have to wait until after the term ends. As @aTrotier notes, it may suffice to simply have an ISMRMRD reader if there are already existing open-source converters.

Jakob, I've invited you to the organization so you can contribute easily. At some point it could make sense to add https://github.com/JakobAsslaender/BartIO.jl to the collection if you want.

I'm flexible about repo/package names, though "MRIFiles" might lead people to think there will be images there and I assume we are just discussing raw data. Probably if the README just points to https://github.com/JuliaHealth/DICOM.jl then it will steer people the right way.

MIRTio is barely used and I am happy to deprecate it eventually.

tknopp commented 2 years ago

Thanks for the feedback. Not careing about Philips/Siemens/GE ... and relying on the ISMRMRD has been my original plan and if the existing converters work good than I don't see a pressing need re-implementing stuff. So, lets do it that way: We introduce MRIFiles.jl which will have some sub packages for vendor file formats (ISMRMRD.jl, BrukerFiles.jl, ... ). And yes, I am talking about raw data exclusively. For recon data we have DICOM.jl and NIfTi.jl.

JakobAsslaender commented 2 years ago

Hi, I think this is a great plan and I am happy to move BartIO.jl. I agree with Jeff's concerns about the naming though: MRIFiles.jl suggests that we are providing files. And I think xIO.jl is a widespread naming convention in the Julia ecosystem, which I think we should stick to.

My two cents regarding vendor formats vs. ISMRMD/MRD format: I myself actually do not use MRD and I know many people who do not. In my case it is foremost for one reason: MRD does not maintain all header information. As we acquire rather large datasets, I do not want to store multiple versions. At the same time, I do not want to delete the original header as I frequently find myself wanting to dig out some obscure header information that I did not think of before. Another reason for me is how difficult it is to install the MRD converters.

So long story short: I would love to have a native reader for Siemens files! I am currently using pymapvbvd that I call via PyCall.jl, which is easy enough to set up, but comes with some severe limitations. So I would love to see a native implementation, but not sure that I will have the time myself to look into into any time soon. An alternative would be to build a package that uses pymapvbvd and maybe finds ways around those limitations. Happy to discuss this in more detail, but didn't want to bore anyone here...

Best, Jakob

aTrotier commented 2 years ago

What are the limitation with pymapvbvd ?

BartIO.jl is still under development (But I am using my fork in multiple Pluto notebook to compare MRIReco and BART reconstruction) but it makes sense to regroup it :)

Should we add a nifti export in that package ? (it will be closely related to the MRD format in order to fill the orientation metadata)

mrikasper commented 2 years ago

Hello everyone,

Thanks for looping me in, @tknopp ! It's interesting to see how workflows diverge re: (ISMR)MRD, because what @aTrotier wrote resonates with me as well, i.e.,

This comes pretty close to our own workflow as well. For myself I previously used Philips. Now, together with @alexjaffray, we have implemented our first recon pipeline for Siemens data using siemens_to_ismrmrd, custom Julia code (e.g., to merge custom gradient waveform text files) and MRIReco.jl. Our main reasons to do so are that we

  1. Often merge raw data components from different vendors (e.g., k-space coil data from Philips and k-space trajectory or higher order encoding field data monitored with @SkopeMagneticResonanceTechnologies systems) into a new raw data file (ISMRMRD)
  2. Manipulate the raw data in different stages (e.g., change a nominal spiral trajectory into one predicted by the gradient impulse response function @MRI-gradient), aiming at using ISMRMRD (or RawAcquisitionData) as an interface, to allow comparing recon pipelines easily
  3. Want to make raw data publicly available for open, reproducible science (e.g., https://doi.org/10.3929/ethz-b-000487412) and retain full control over what header information is in there (e.g., remove all subject-specific parts).

So I can see that one wants to keep the vendor-specific data internally to have all acquisition info for future reference, and I can second @JakobAsslaender in that the data storage requirements are huge otherwise, but I feel that for interoperable workflows, (ISMR)MRD is the way to go, as well as using their converters (siemens_to_ismrmrd etc.). A huge advantage of that approach is that we can join forces with the @Gadgetron community who relies on that format, given that we all have limited resources.

It's probably instructive to dig deeper why there is a certain reluctance against using MRD and its converters, e.g.,

  1. Are their downsides of a Julia-external conversion?
  2. What are the installation hurdles with ISMRMRD converters @JakobAsslaender mentioned (I could imagine that installing Matlab or Python just for a data conversion with mapVBVD could also be a hurdle for some)?
  3. Are there any cases where <vendor>_to_ismrmrd doesn't work as expected and we could rather fix the problem there than reimplementing the conversion in Julia?
  4. If we do manipulate raw k-space data within Julia, and compare its impact on recon pipelines, what interfacing file format would we choose, if not MRD?

Re: Raw vs Image data: For image data (which can also be "raw" in the sense of input data, e.g., sensitivity or B0 maps), I usually resort to nifti, because it plays so nicely with all neuroimaging packages for image viewing/manipulation. Not sure whether MRD should really try to be an image format as well, at the moment it seems exotic, but I would like to learn about who uses it in which circumstances.

All the best, Lars

aTrotier commented 2 years ago

So I can see that one wants to keep the vendor-specific data internally to have all acquisition info for future reference, and I can second @JakobAsslaender

I guess what is missing is a way to export in json files all the parameters from the vendor-specifc data rather than only the ones defined in the parameter maps.

Re: Raw vs Image data: For image data (which can also be "raw" in the sense of input data, e.g., sensitivity or B0 maps), I usually resort to nifti, because it plays so nicely with all neuroimaging packages for image viewing/manipulation. Not sure whether MRD should really try to be an image format as well, at the moment it seems exotic, but I would like to learn about who uses it in which circumstances.

I have some codes that i can use to create a NIFTI from raw mrd/img mrd but because it is a homemade custom convertor I am not really sure about the results and I prefer to keep the img.h5 files (which also stores some parameters).

JakobAsslaender commented 2 years ago

Don't get me wrong: I love the idea of MRD; the advantage in practice and in my own use case was just too small to deal with mentioned issues. But as @aTrotier mentioned: If there was a way to dump the original header in the MRD format, in addition to the nicely organized MRD header itself, it would help towards guaranteeing that nothing is lost by converting the data (rather than storing a second copy).

Regarding the installation hurdles, I was mostly referring to the usual problems of C: E.g., the installation guidelines quite literally start with sudo and many people don't have root privileges on their servers and HPC clusters. Plus, it is platform specific. In comparison, a Julia package that is installed with its own package manager works just so much more robust and easily on all platforms, as does python or Matlab.

tknopp commented 2 years ago

This is all very important feedback and while there is quite some diversity in the workflow and the use cases (as expected), I still think that we can come up with a unified Julia package that addresses various workflows. Initially we will go for MRD+BrukerFiles which is a nice playground. But I feel with @JakobAsslaender that we should keep this open to cover also more vendor file formats. I know its tedious work but often it is worth going the extra mile to have something that works out of the box. So if your use case is to not convert but directly load siemens data then we should target that goal.

My take away from this thread is that a modularization of MRIReco.jl is what people would appreciate. I need to think about weather to put things into MagneticResonanceImaging or JuliaImageRecon and this package needs a name MRIFiles.jl or MRIIO.jl or MRIFileIO.jl are some ideas.

aTrotier commented 2 years ago

If someone is working with small vendor ( ex : MRSolution) or even custom MRI (we have an earth field scanner in my lab), they only have to write a reader and after that he can easily also use MRIReco as a MRD convert if for example he wants to upload the data to http://mridata.org/

Le lun. 7 févr. 2022 à 18:44, Tobias Knopp @.***> a écrit :

This is all very important feedback and while there is quite some diversity in the workflow and the use cases (as expected), I still think that we can come up with a unified Julia package that addresses various workflows. Initially we will go for MRD+BrukerFiles which is a nice playground. But I feel with @JakobAsslaender https://github.com/JakobAsslaender that we should keep this open to cover also more vendor file formats. I know its tedious work but often it is worth going the extra mile to have something that works out of the box. So if your use case is to not convert but directly load siemens data then we should target that goal.

My take away from this thread is that a modularization of MRIReco.jl is what people would appreciate. I need to think about weather to put things into MagneticResonanceImaging or JuliaImageRecon and this package needs a name MRIFiles.jl or MRIIO.jl or MRIFileIO.jl are some ideas.

— Reply to this email directly, view it on GitHub https://github.com/MagneticResonanceImaging/MRIReco.jl/issues/66#issuecomment-1031741278, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC5P7O3JH5HIXN2XHIR6IW3U2AAGVANCNFSM5NVKLOJA . You are receiving this because you were mentioned.Message ID: @.***>

--

TROTIER Aurélien 31 allée de la Biotte 33470 Gujan-Mestras 06.09.12.61.61 @.***

alexjaffray commented 2 years ago

Thanks @tknopp for opening this discussion and @mrikasper for the mention! I think my sentiments on the topic have been well covered by Lars' comment, however I would like to additionally echo what @JakobAsslaender mentioned about the installation hurdles and add some further motivation for Julia-native converters.

From my own experience, following the installation steps for ISMRMRD and the vendor-specific converters on an unmanaged computer is a reasonable task for someone familiar with building projects from source. However, on a managed system, the need for sudo privileges (or the need to know how to work around them) can present a barrier to researchers. Gadgetron itself gets around this by promoting a containerized deployment approach, which is reasonable for their application, but is a bit overkill for many researchers.

Also, is there a good place in the Julia MRI landscape where we can point users to vendor-specific reconstructed data handling packages? This would aid in comparison between vanilla scanner reconstructions and those done using offline reconstruction tools like MRIReco.jl. Perhaps it is worth starting a discussion page to curate such tools. Christian Kames (@kamesy), a colleague of mine at the UBC MRI Research Center, has developed and registered a Par/Rec reader for Julia (ParXRec.jl), and I would think there are other such tools out there.

tknopp commented 2 years ago

Wow, very cool that we have a ParRec package! Thanks for the feedback Alex.

In my opinion we should gather all these packages in one Github organization. That is actually the purpose of MagneticResonanceImaging. For general discussions on the big picture we can use the discussion forum in MRIReco.jl. So that is the place where we discuss how we structure packages and so on.

I personally like having the MRI parts in a dedicated MRI organization. We have the same for MPI (https://github.com/MagneticParticleImaging) and this works very well. I see and like the point of JuliaImageRecon but that in my opinion could be the host for general imaging-agnostic-reconstruction algorithms.

tknopp commented 2 years ago

72 is an attempt to split MRIReco into pieces.

tknopp commented 2 years ago

Since #72 is merged, this issue can be closed.