hMRI-group / hMRI-toolbox

A toolbox for quantitative MRI and in vivo histology using MRI (hMRI)
GNU General Public License v2.0
60 stars 44 forks source link

Char incompatibility size #107

Open franciscofritz opened 1 week ago

franciscofritz commented 1 week ago

I cannot calculate the MPM parameters because it is a problem with the vertical concatenation of the my MPM contrast files.

Specifically, this is my input of my MPM contrast files:

inputs{6,crun} = cellstr(["D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00036_FID175779_fj_mfc_3dflash_MT_1p0_R25_rec-loraks_echo-1_mt-on_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00036_FID175779_fj_mfc_3dflash_MT_1p0_R25_rec-loraks_echo-2_mt-on_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00036_FID175779_fj_mfc_3dflash_MT_1p0_R25_rec-loraks_echo-3_mt-on_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00036_FID175779_fj_mfc_3dflash_MT_1p0_R25_rec-loraks_echo-4_mt-on_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00036_FID175779_fj_mfc_3dflash_MT_1p0_R25_rec-loraks_echo-5_mt-on_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00036_FID175779_fj_mfc_3dflash_MT_1p0_R25_rec-loraks_echo-6_mt-on_part-mag.nii,1"]);
inputs{7,crun} = cellstr(["D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00034_FID175777_fj_mfc_3dflash_PD_1p0_R25_rec-loraks_echo-1_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00034_FID175777_fj_mfc_3dflash_PD_1p0_R25_rec-loraks_echo-2_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00034_FID175777_fj_mfc_3dflash_PD_1p0_R25_rec-loraks_echo-3_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00034_FID175777_fj_mfc_3dflash_PD_1p0_R25_rec-loraks_echo-4_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00034_FID175777_fj_mfc_3dflash_PD_1p0_R25_rec-loraks_echo-5_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00034_FID175777_fj_mfc_3dflash_PD_1p0_R25_rec-loraks_echo-6_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00034_FID175777_fj_mfc_3dflash_PD_1p0_R25_rec-loraks_echo-7_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00034_FID175777_fj_mfc_3dflash_PD_1p0_R25_rec-loraks_echo-8_mt-off_part-mag.nii,1"]);
inputs{8,crun} = cellstr(["D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00032_FID175775_fj_mfc_3dflash_T1_1p0_R25_rec-loraks_echo-1_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00032_FID175775_fj_mfc_3dflash_T1_1p0_R25_rec-loraks_echo-2_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00032_FID175775_fj_mfc_3dflash_T1_1p0_R25_rec-loraks_echo-3_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00032_FID175775_fj_mfc_3dflash_T1_1p0_R25_rec-loraks_echo-4_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00032_FID175775_fj_mfc_3dflash_T1_1p0_R25_rec-loraks_echo-5_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00032_FID175775_fj_mfc_3dflash_T1_1p0_R25_rec-loraks_echo-6_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00032_FID175775_fj_mfc_3dflash_T1_1p0_R25_rec-loraks_echo-7_mt-off_part-mag.nii,1";
                          "D:\Patients\derivatives\InVivo\ReconLeipzig\sub-tle001\ses-preop\anat\meas_MID00032_FID175775_fj_mfc_3dflash_T1_1p0_R25_rec-loraks_echo-8_mt-off_part-mag.nii,1"]);

And the error appears in line 1185 of the code "hmri_create_MTProt (internal function: get_mpm_params)": raw = [jobsubj.raw_mpm.MT; jobsubj.raw_mpm.PD; jobsubj.raw_mpm.T1];

Where now the jobsub.raw_mpm structures are char structure are 6x162, 6x163 and 6x163; I can see the problem. What I don't understand is why it is forced that the inputs must be the same character size rather than just strings. Any suggestion (that is not changing the names) would be appreciated.

lukeje commented 1 week ago

You need to use char arrays, not strings. You can use the char command to switch between the two.

lukeje commented 1 week ago

Ah, I see that cellstr also does that. What is the actual error you get?

lukeje commented 1 week ago

@Barisevrenugur I see that the line in question is from your orientation check code: https://github.com/hMRI-group/hMRI-toolbox/blob/a03439b42b3a639f50fdea8283e2860e6b91c43e/hmri_create_MTProt.m#L1185C8-L1185C26 I think it does have a bug if the MTw, PDw, and/or T1w files have different file name lengths. Could you test it (e.g. with a PDw file named PDw.nii and a T1w file named T1w_longerfilename.nii) and see if you reproduce the error?

franciscofritz commented 1 week ago

The exact error I get is: "error in vertcat: array sizes are not compatible for concatenation" (which I can see why does not work) - I had to do a quick dirty fix in that code line (1185) - defined again as a cellstr: raw = [cellstr(jobsubj.raw_mpm.MT); cellstr(jobsubj.raw_mpm.PD); cellstr(jobsubj.raw_mpm.T1)]; and that makes everything to run smoothly

lukeje commented 1 week ago

How are you calling hmri_create_MTProt? As far as I can tell jobsubj.raw_mpm.MT, etc. should already be cell strings not char arrays.

Barisevrenugur commented 1 week ago

Dear @franciscofritz, this line of code that i believe i implemented during hackathon was already tested but possibly with file names with the same length (but then as far as i checked now, all sets of test data that I posses do have the same file name length!). With @lukeje , we made a quick test with files having different name lengths but the toolbox executed without any problem, we did not reproduce the problem you reported above.

As we discussed, this happens (file names with differing length) particularly with the BIDS format of the MT file names which you also have above as example:

the files otherwise having names with the same length end up having names of different lengths due to addition of on (2 characters) and off (3 characters) to the regular MT (same-length) file names.

franciscofritz commented 2 days ago

Thanks for your answers. Let me show you my problem step by step using only the GUI. The images (MT, PD and T1) are loaded accordingly as you can see here: For MT: image For PD: image For T1: image

Now if I execute the batch using the hMRI toolbox, I get this error: image

But I found something interesting... this only happens IF I am doing RF sensitivity bias correction per contrast. If I don't do this, then it goes without any error!

lukeje commented 2 days ago

That's very interesting; it seems like the rfsens code converts to char, which could explain the problem. However, I thought that the standard testing used per-contrast rfsens correction and so this should have been caught.

@Barisevrenugur Could you confirm whether your testing includes per-contrast rfsens correction, and if not, see whether you can reproduce Frank's problem?

lukeje commented 2 days ago

Ah, the confounding factor is that all our filenames in the testing are the same length. Therefore the stacking of char arrays coincidentally works. The choice would be between fixing the problem in hmri_create_MTprot or in hmri_create_RFsens.

franciscofritz commented 2 days ago

Yeah, for now I did a quick hack in hmri_create_MTprot for now.

Barisevrenugur commented 1 day ago

But I found something interesting... this only happens IF I am doing RF sensitivity bias correction per contrast. If I don't do this, then it goes without any error!

the line you reported at the beginning:

in line 1185 of the code "hmri_create_MTProt (internal function: get_mpm_params)":
raw = [jobsubj.raw_mpm.MT; jobsubj.raw_mpm.PD; jobsubj.raw_mpm.T1];

works without any problem with same-length and differing-length filenames, as tested manually by me and also by me/Luke as I wrote above.

in the below output (manual test): the beginning is the content of raw in the succesfull testing with different-length file names and below line shows that raw is a 8x1 cell array-as intended, not a char array):

'pathto\pdw_mfc_3dflash_v1i_R4_0009\anon_s2018-02-28_18-[26-185345-00001-00224](callto:26-185345-00001-00224)-1.nii,1'
'pathto\pdw_mfc_3dflash_v1i_R4_0009\anon_s2018-02-28_18-[26-185345-00001-00448](callto:26-185345-00001-00448)-2.nii,1'
'pathto\pdw_mfc_3dflash_v1i_R4_0009\anon_s2018-02-28_18-[26-185345-00001-00672](callto:26-185345-00001-00672)-3.nii,1'
'pathto\pdw_mfc_3dflash_v1i_R4_0009\anon_s2018-02-28_18-[26-185345-00001-00896](callto:26-185345-00001-00896)-4.nii,1'
'pathto\t1w_mfc_3dflash_v1i_R4_0015\anon-1.nii,1'
'pathto\t1w_mfc_3dflash_v1i_R4_0015\anon-2.nii,1'
'pathto\t1w_mfc_3dflash_v1i_R4_0015\anon-3.nii,1'
'pathto\t1w_mfc_3dflash_v1i_R4_0015\anon-4.nii,1'

1185 raw = [jobsubj.raw_mpm.MT; jobsubj.raw_mpm.PD; jobsubj.raw_mpm.T1];
K>> whos raw
  Name      Size            Bytes  Class    Attributes

  raw       8x1              3064  cell

the toolbox is tested both with per contrast and also US. So, we do test it with per contrast RFsens correction as well, however our tests were succesfull.

I did not check it specifically for RFsens maps but yes (as I clarified above) all sets of test data we possess have same length names: this is a standard, there is nothing wrong with it.

The line of code (the MTprot line above works fine with different length file names- as tested several times) that would lead to the above problem with differing file names is this one in RF sensitivity correction (as you also quoted above):

https://github.com/hMRI-group/hMRI-toolbox/blob/ffa84442cce98527a7aa20756133343a8e3f8b4d/hmri_create_RFsens.m#L98

This I have not directly/manually tested yet but can say rightaway that it would cause the above quoted problem. The simple solution would be to change the char array on this line with a cell array which is congruent with the MTprot code. It is OK to change this because jobsubj.raw_mpm.(rfsens_params.input(ccon).tag) appears only in one place, as far as I searched the code now.

lukeje commented 1 day ago

@Barisevrenugur Please try and replicate the issue before changing the code; if you can't replicate it, then you can't test whether any fix works. I would suggest running the toolbox with the BIDS-ified version of the test dataset (dsm-mpm here), as there the MT-weighted data should have different length filenames compared to the PD- and T1-weighted data.

@franciscofritz Could you make a pull request with your hack so we can test it?