Added siemens data reader and phase shifter #89
Conversation
src/mrinufft/io/siemens.py
Outdated
| import numpy as np | ||
|
|
||
| try: | ||
| from mapvbvd import mapVBVD |
There was a problem hiding this comment.
This needs to also be added to pyproject.toml (maybe in an io dependency bundle ?)
src/mrinufft/io/siemens.py
Outdated
| if not MAPVBVD_FOUND: | ||
| raise ImportError( | ||
| "The mapVBVD module is not available. Please install it using " | ||
| "the following command: pip install pymapVBVD" | ||
| ) |
There was a problem hiding this comment.
you may simply do the import in the function then ?
src/mrinufft/io/siemens.py
Outdated
| "num_coils": int(twixObj.image.NCha), | ||
| "num_shots": int(twixObj.image.NLin), | ||
| "num_contrasts": int(twixObj.image.NSet), | ||
| "num_adc_samples": int(twixObj.image.NCol), | ||
| "num_slices": int(twixObj.image.NSli), |
There was a problem hiding this comment.
overall in MRI-NUFFT we tend to use n_coils, n_shots, etc... I would advise to use these convention here too.
src/mrinufft/io/siemens.py
Outdated
| hdr["num_slices"], | ||
| hdr["num_contrasts"] | ||
| ) | ||
| if "SPARKLING_VE11C" in data_type: |
There was a problem hiding this comment.
do you have other value of data_type in mind ?
I would recommend to use an enum to gather them. Overall this section of the code only updates the header, and may be refactored to a dedicated function. The workflow would then be
parser = parse_header_type[data_type] # e.g. using a `MethodRegister` from `_utils.py`
hdr |= parser(twixObj)
src/mrinufft/io/utils.py
Outdated
| @with_numpy_cupy | ||
| def add_phase_to_kspace_with_shifts(kspace_data, kspace_loc, normalized_shifts): |
There was a problem hiding this comment.
with_numpy_cupy expect a signature with (self, data, output=None). I think you are going to get trouble here.
|
Adding back for review of updated codes. I will get some tests up also though |
Co-authored-by: Pierre-Antoine Comby <pierre-antoine.comby@crans.org>
|
There seems to be no way to test IO arbGrad module. For now lets just merge and add an issue for needing test? |
|
Something went wrong here! I dont see my changes at master. The split up of read_siemens_rawdat and read_arbgrad_rawdat stuff. I will change this in #90 PR. |
These essential functions seem to be better if it belonged here as compared to on
pysap-mri. That way, you can have a complete DC Adjoint code working from CLI without any need for pysap-mri, for light simple quick rersults..Later,
pysap-mrican build further ahead on this CLI for compressed sensing based recon. I have basic CLI code setup and I shall make another PR for it. Still figuring out the structuring of config.