AcquisitionHeader Data Type Question

[alexjaffray]

I noticed that the AcquisitionHeader stores the number of samples as an Int16. Is this done for speed or memory reasons? Would there be a problem with changing it to UInt32, or even UInt16?

The reason I am asking is that this datatype choice limits the number of samples in a given readout to 32767. Most analog readouts have a dwell time of around 2 microseconds, and so this limit corresponds to a readout time of around 60ms. While this is close to the practical limit for readout times of single-shot high resolution imaging, the dwell times of ADC-on-coil systems (such as the newer Philips hardware) can be significantly shorter. If they are even a factor of 2-4 shorter, the readout time limitation would prevent successful reconstruction of classical single-shot acquisitions, such as those by myself and @mrikasper.

I only noticed this as I am in the process of setting up a reconstruction pipeline for Philips data using MRIReco.jl and wanted to see if there was enough headroom to reconstruct long readout acquisitions at high sampling rates.

Also, is a Github issue the proper place to bring up a "design" question? I can move it to a discussion post if that is a better location.

Thanks, Alex

[tknopp]

Also, is a Github issue the proper place to bring up a "design" question? I can move it to a discussion post if that is a better location.

There is no rule for that, but things like this can be kept as issues. The discussion section is more for usage questions. Before we had that it was a little bit odd to open issues when one just had a question. Your point here is not a user question but a design discussion. So it fits well.

[tknopp]

Regarding the question: In our design right now we have a 1-to-1 correspondence between ISMRMRD and RawAcquisitionData. We could at some point separate this but it raises new challenges since we then need to think about how we can load and store our own internal representation into ISMRMRD. So changing to UInt32 is not completely trivial.

But: Actually I don't know why I used the signed integer there. If you look here: https://github.com/ismrmrd/ismrmrd/blob/master/include/ismrmrd/ismrmrd.h you can see that it actually should be unsigned values most of the times.

If that would fix your concrete issue with the Philips data, I would propose that we do this in the first step.

[alexjaffray]

I agree, changing to UInt16 to be consistent with ISMRMRD would be a good idea (regardless of whether my issue is fixed), and it also would fix my issue in the interim. I am just getting this project off of the ground so it will be a while until I determine whether there is a practical need to go beyond UInt16.

From a design paradigm point of view, I very much like the idea of keeping a 1:1 correspondence between RawAcquisitionData and ISMRMRD. It is easy to understand, maintain and extend. Given that, I think it would be reasonable to bring up the change with the ISMRMRD community should it become an obvious limitation, and then simply adjust the RawAcquisitionData to match whatever they decide, keeping the 1:1 correspondence.

[tknopp]

Perfect. Sounds like a plan. I share the opinion that we should first discuss with ISMRMRD if there might by a limitation in the file format instead of working around it. So please open an issue there. Certainly it makes sense to do this well prepared, probably with some actual Philips files that showcase the limitation.

In the mean time I will have a look at the unsigned integer change.

[alexjaffray]

Great! I will be aware of the potential limitation and acquire some data in the next month or so to see if it is a problem in practice and add a well-informed issue in ISMRMRD should that be the case.

• edited

[tknopp]

The switch to unsigned int went smoothly:

https://github.com/MagneticResonanceImaging/MRIReco.jl/commit/af51715171d1e5d213128be35656c0bf8106563b

[alexjaffray]

For the time being, this issue is solved for me, so I am closing this. I will open an issue with ISMRMRD once we have a concrete example as discussed in this thread.