-
Notifications
You must be signed in to change notification settings - Fork 9
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
simulating MRI signal #55
base: main
Are you sure you want to change the base?
Conversation
added forward model for C to R1 added example in user guide for simulating an MRI signal
I think this looks great, my only thought is whether it would be useful to comment the examples to state exactly what is being done at each step, what the units are for the input parameters etc to kind of lead users through? I think you could work it out by reading alongside the documentation and we don't want to duplicate work so I'm in two minds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In line with what Lucy mentioned, I've left some suggestions you should be able to apply directly. Otherwise looks good.
import numpy as np | ||
import matplotlib.pyplot as plt | ||
import osipi | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Compute Parker AIF and Simulate Tissue Signal |
Ktrans = 0.6 | ||
ve = 0.2 | ||
ct = osipi.tofts(t, ca, Ktrans=Ktrans/60, ve=ve) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Set Relaxivity Constants (Maybe include the agent you are assuming here) and Compute Longitudinal Relaxation Rate |
R10 = 0.5 | ||
r1 = 4.5 | ||
R1t = osipi.C_to_R1_linear_relaxivity(ct, R10, r1) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Compute MR signal using Relaxation Rate, Initial signal, Repetition Time, and Flip Angle. |
Although maybe not directly related we should probably discuss: Rather than assuming signal_SPGR is the T1 weighted approximation, we should probably make the function general, and have it break down to it's simpler forms based on an argument. If not, we should at least indicate that this model assumes the T2* effects are negligible (maybe in the actual function, rather than in the example...
TR = 0.004 | ||
a = 12 | ||
St = osipi.signal_SPGR(R1t, S0, TR, a) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Display |
Notes on function signal_SPGR and others:
1) you are declaring variables as float64 but this seems unnecessarily
restrictive. Your function will work fine if they are float32 as well for
instance. Also your code will work fine of R1 is a numpy array, which is in
fact the case in your examples. Declaring variables explicitly as
np.float64 indicates to users that variables need to be converted in this
type for the function to work. This might consume unnecessary memory for
those who work with integer of float32 images. It also suggest that a loop
is required to determine the signal for all elements of an array R1.
Suggestion: use numpy conventions for type declarations (throughout the
whole package):
R1: array-like
S0, TR, a: float (without specifying the size)
2) the function signal_SPGR returns the *steady state* signal of the SPGR
sequence. Are you planning to include a function for the more general case
at some point in the future (outside the steady state)? If so it might be
more logical for this function to be called something else (e.g.
signal_SPGRSS) and reserve the name signal_SPGR for the more general case.
3) small error in docstring: S0 is *not* the fully relaxed signal
(S0*sin(a) is the fully relaxed signal)
Prof. Steven Sourbron, PhD
Chair in Medical Imaging Physics
University of Sheffield, UK
*https
<https://www.sheffield.ac.uk/medicine/people/iicd/steven-sourbron>://www.sheffield.ac.uk/medicine/people/iicd/steven-sourbron
<https://www.sheffield.ac.uk/medicine/people/iicd/steven-sourbron>*
*Sheffield abdominal imaging research page*
<https://www.sheffield.ac.uk/medicine/research/research-themes/medical-imaging/medical-imaging-research/abdominal-imaging>
*Open source initiative in perfusion imaging* <http://www.osipi.org>
*Magnetic resonance imaging biomarkers for chronic kidney disease
<https://renalmri.org/>*
…On Wed, 11 Dec 2024 at 17:56, Luis Torres ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In docs/user-guide/gen_aif.md
<#55 (comment)>:
> +
+t = np.arange(0, 6*60, 1)
+ca = osipi.aif_parker(t)
+Ktrans = 0.6
+ve = 0.2
+ct = osipi.tofts(t, ca, Ktrans=Ktrans/60, ve=ve)
+
+R10 = 0.5
+r1 = 4.5
+R1t = osipi.C_to_R1_linear_relaxivity(ct, R10, r1)
+
+S0 = 1
+TR = 0.004
+a = 12
+St = osipi.signal_SPGR(R1t, S0, TR, a)
+
⬇️ Suggested change
-
+# Display
—
Reply to this email directly, view it on GitHub
<#55 (review)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABOFKA6RUI57Q2HTV72RHT32FB4D5AVCNFSM6AAAAABS4YQF2KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIOJWGQZTKNZVGE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
added forward model for C to R1
added example in user guide for simulating an MRI signal
this step is needed to them implement the example for fitting the data to a tofts model.