-
Notifications
You must be signed in to change notification settings - Fork 44
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
PD calculation without OLS broken if nr_echoes_forA
does not equal nr_echoes4avg
#87
Comments
@mfcallaghan @ChristophePhillips @antoinelutti Do you have a strong opinion which solution would be best? |
@lukeje Honestly, no idea...
|
@ChristophePhillips There were a couple of overlapping reasons for the code to change:
In principle 2. and 3. could also have been done within the old approach (rewriting the PD calculation function as necessary), but 1. is something that we can't easily get around. Pros and cons of the different solutions to follow in the next comment... |
As promised, here's a list of what I see as the pros and cons of the different options. I'm ignoring here that only option C (or option B modified in line with C) would allow for different echo times between the contrasts (rather than just different numbers of echoes acquired with the same echo spacing), as I think that is a separate issue. A: Remove
|
Hi @lukeje , @ChristophePhillips and @mfcallaghan |
unfortunately, I think I (still) did not understand this one. But I came across the following lines in create_MTprot which seems related: hMRI-toolbox/hmri_create_MTProt.m Line 1399 in 894a38b
and hMRI-toolbox/hmri_create_MTProt.m Line 1220 in 894a38b
now, in the code following L1399 and in the code following L1220 it simply hMRI-toolbox/hmri_create_MTProt.m Line 1218 in 894a38b
why do not the code following the above hinted lines fix the problem of non-equal number of echoes for averaging? but they seem to? with respect to processing, they happen pretty early in the code, namely already in get_mpm_params, which reads off the params at the beginning of mt_prot |
Because we are talking about |
I guess this is an alternative "E" closely related to "A": rather than defining |
Background
In versions prior to v0.2.5, the PD (A) map was computed using R1 (rather, T1) and the T1w data (i.e. the PDw data was only used in the calculation of the R1 map). The map calculation was implemented in such a way as to allow a different number of echoes to be used for the A calculation using the
hmri_def.PDproc.nr_echoes_forA
variable if extrapolation to TE=0 was turned off. As only the T1w data and not the PDw data was needed, a specialT1w_forA
volume was created which was averaged overnr_echoes_forA
echoes and input into the A calculation.Problem
From v0.2.5 onwards, the PD calculation uses both T1w and PDw data. Therefore in order to correctly account for R2* effects, the PDw data would also need to use only
nr_echoes_forA
echoes if extrapolation to TE=0 was turned off.Possible fixes
nr_echoes_forA
entirely and just use the averaged PDw, T1w and MTw used for registration (which averages the same number of echoes for all contrasts)PDw_forA
(andMTw_forA
) which are also averaged overnr_echoes_forA
and used for A calculationT1w_forA
data separately rather than correcting A, using the echoes actually used for each averageThe text was updated successfully, but these errors were encountered: