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

Enhance support for GE data #132

Merged
merged 20 commits into from
Mar 21, 2024
Merged

Enhance support for GE data #132

merged 20 commits into from
Mar 21, 2024

Conversation

markmikkelsen
Copy link
Contributor

This PR includes changes to code for converting GE (P-file) datasets to the NIfTI-MRS format. I have tested these changes on multiple datasets of various types, including the MEGA-PRESS and PRESS data from the Big GABA dataset.

Changes include:

  • Adding missing header fields for revisions 14, 15, 16
  • Cleaner coding of functions to load specific psd's
  • Adding support for several new pulse sequence variants
  • Several spelling/grammar corrections
  • Several cosmetic changes
  • Bug fixes

Add an if-then statement for sLASER datasets where the receiver phase was set
Added missing header fields for revisions 14, 15, and 16
Cosmetic edits to code
- Cleaned up code a bit in `ge_read_pfile.py`
- Added support for a few other psd's
The last block of lines was previously within the `else` statement just above, which prevented those lines from running when `nechoes == 1`
Incorrect header fields were being used to determine `dataframes` and `refframes`
- Updated the `mult` and `multw` factors to match the current ones used in Gannet [NOTE: These have still not been completely finalized and may need to be adjusted in the future]
- Some cosmetic edits
@wtclarke
Copy link
Owner

Amazing! Thanks so much @markmikkelsen. Is there a list of sequences or data we should add to the test set? I'm keen to do this now so that we protect against regressions later (I've got a couple of other bits and pieces for GE to do at some point).

@markmikkelsen
Copy link
Contributor Author

@wtclarke I could add some of the Big GABA GE data. However, a number of the datasets I tested these changes on cannot be shared. How do I go about adding the ones I can share?

@wtclarke
Copy link
Owner

wtclarke commented Mar 13, 2024 via email

@markmikkelsen
Copy link
Contributor Author

Will send you a link via email :)

@wtclarke
Copy link
Owner

Hi @markmikkelsen, I got the data successfully. For the various G{N} in the Big GABA datasets, is there any substantial difference? Or could I test everything of note by just including one of each of the PRESS and MPRESS?

@markmikkelsen
Copy link
Contributor Author

markmikkelsen commented Mar 19, 2024

Hi @wtclarke, there are some subtle differences between the datasets related to the header revision number, how the data were encoded (e.g., whether or not the FIDs were summed over phase cycle), and the name of the PSD. A number of my edits help tease out these nuances.

@wtclarke
Copy link
Owner

Good to know. I'll write some tests on this branch and we can check that it captures all the salient points then.

@wtclarke
Copy link
Owner

image What's the size two dimension in the sub-01_ses-01_acq-press_svs_noID.7 data? Is it just two blocks of ADCs with different phase settings? Would `DIM_PHASE_CYCLE` be a better dimension flag than `DIM_EDIT`?

@wtclarke
Copy link
Owner

wtclarke commented Mar 20, 2024

Oh I see that the PRESS data that has the DIM_EDIT is using the jpress sequence, rather than probe-p. Was this a hack in the data collection? E.g. for sequence availability? Does it just set the edit pulses off in some way?

For the edited data, is there a way to read the editing frequency / pulse info from the p-file headers? It's nice to stick it in the NIfTI headers if it's there.

@markmikkelsen
Copy link
Contributor Author

Was this a hack in the data collection? E.g. for sequence availability? Does it just set the edit pulses off in some way?

Correct; the jpress PSD was used. One can turn off editing using this PSD by setting numecho to 1 on the scanner (the edit RF waveform is then simply 0). However, I now realize the consequence is that the DIM_EDIT field will be filled by spec2nii, meaning a correction for every other FID would have to be made somewhere in one's data loader.

For the edited data, is there a way to read the editing frequency / pulse info from the p-file headers?

Yes, but only the MEGA editing approaches provide accurate values. The HERMES/HERCULES approaches read in this info via a text file on the scanner that the user can manipulate. Unfortunately, these are (currently) not stored in the header. That said, they may be in more recent header revision versions/software releases, but we would need input from Ralph Noeske to be sure.

But this is how one would get such info (adopted from GERead.m in Gannet):

edit_rf_waveform = self.hdr.rhi_user19
edit_rf_freq_off1 = self.hdr.rhi_user20
edit_rf_freq_off2 = self.hdr.rhi_user21
edit_rf_dur = self.hdr.rhi_user22
# RTN 2018: check for default value (-1) of pulse length
if edit_rf_dur <= 0:
    edit_rf_dur = 16000

@wtclarke
Copy link
Owner

Is the numecho parameter the same as self.hdr.rhi_numecho? If so we could easily special case this and 1) apply a 180 degree phase to half the data and 2) unravel that dimension so there is only DIM_DYN. Would that make sense?

Worth getting this right now, so I'll drop Ralph an email (with you cc'd) to see if there is a clear answer on the last point.

@markmikkelsen
Copy link
Contributor Author

Is the numecho parameter the same as self.hdr.rhi_numecho?

That's correct; yep.

If so we could easily special case this and 1) apply a 180 degree phase to half the data and 2) unravel that dimension so there is only DIM_DYN. Would that make sense?

Makes sense! I think I tested something like that out, but I forgot that in the NIfTI-MRS reader, it didn't take into account that 180-deg phase difference.

@wtclarke
Copy link
Owner

Ok, I've implemented the fix as you suggested. I'll wait to hear from Ralph to work on the editing pulse information.

@wtclarke wtclarke merged commit e993d46 into wtclarke:master Mar 21, 2024
4 checks passed
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 this pull request may close these issues.

2 participants