Skip to content

Noise #26

Merged
merged 24 commits into from Dec 5, 2012

7 participants

@arokem
NIPY developers member
arokem commented Jun 25, 2012

This pull request refactors the noise-generating functions in the simulation module. I have written down precisely how I define SNR for the purpose of this. The one thing that I am feeling unsure about is the definition of the Rician noise from two Gaussians. My idea there follows up on Bago's suggestion on the mailing list.

@MrBago MrBago and 1 other commented on an outdated diff Jun 25, 2012
dipy/sims/phantom.py
+ # all the voxels and with the right shape:
+ noise = np.random.randn(*vol.shape) * (np.sqrt(p_noise))
+ elif noise_type == 'rician':
+ # To generate rician noise, we add two IID Gaussian noise sources in
+ # the complex domain and combine them together:
+ noise1 = np.random.randn(*vol.shape)
+ noise2 = np.random.randn(*vol.shape)
+ noise_initial = np.sqrt(noise1**2 + noise2**2)
+ # Now let's get control of the variance, to make sure that we have the
+ # right power:
+ var_initial = np.var(noise_initial, -1)
+
+ # We will scale a demeaned version of the noise
+ mean_initial = np.mean(noise_initial,-1)[...,np.newaxis]
+ demeaned = noise_initial - mean_initial
+ # By our goal for the variance:
@MrBago
MrBago added a note Jun 25, 2012

If I'm reading this code correctly you're assuming that abs(signal + noise) is the same as signal + abs(noise), which I believe isn't the case. To get a rician distribution I believe you need abs(signal + noise) (otherwise I believe it's a Rayleigh distribution).

@arokem
NIPY developers member
arokem added a note Jun 25, 2012

Which line does this refer to? Both the signal and the noise here (in the Rician case) are supposed to be strictly positive, so, as I understand it abs(signal + noise) is the same as signal + abs(noise).

@MrBago
MrBago added a note Jun 25, 2012

Sorry, this was meant to go on line 199, I'm not sure why it ended up here. signal + abs(complex) is not the same as abs(signal + complex), the mr noise is complex, that is why we end up with a rician distribution.

@arokem
NIPY developers member
arokem added a note Jun 25, 2012

So, if I understand you correctly, you suggest that we corrupt the signal with something like:

signal = signal + np.random.randn(shape) + np.random.randn(shape) * 1j

And then take the absolute value of that as our final signal?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@MrBago MrBago and 1 other commented on an outdated diff Jun 25, 2012
dipy/sims/phantom.py
+ # SNR = var(signal)/var(noise) => var(noise) = var(signal)/SNR:
+ p_noise = np.mean(p_signal/snr)
+
+ if noise_type == 'gaussian':
+ # Generate the noise with the correct standard deviation, averaged over
+ # all the voxels and with the right shape:
+ noise = np.random.randn(*vol.shape) * (np.sqrt(p_noise))
+ elif noise_type == 'rician':
+ # To generate rician noise, we add two IID Gaussian noise sources in
+ # the complex domain and combine them together:
+ noise1 = np.random.randn(*vol.shape)
@MrBago
MrBago added a note Jun 25, 2012

Is there a reason we're getting the variance from the noise sample and not the distribution? This could be a real issue if we're adding noise to a small number of voxels, (will this work for one voxel? will the variance be 0?)

@arokem
NIPY developers member
arokem added a note Jun 25, 2012

Do you mean we should get the distribution from scipy.stats and sample from that? They way I wrote this, we are getting a sample (or samples, for the Rician case) of Gaussian noise and then all the rest is just to get it to have the right variance, so that:

SNR == var(signal)/var(noise)

@MrBago
MrBago added a note Jun 25, 2012

I think we need to distinguish between var(noise-sample) and var(noise-distribution). These two will be very close for a large noise-sample, but at the end of the day, var(noise-sample) is only an estimate of var(noise-distribution) and when noise-sample is small, it's a pretty bad estimate. I believe is usually defined as SNR = mean(signal) / sqrt(var(noise-distribution)). We can either hard code the analytical form for var(rice-distribution) for use scipy.stats either should work. Also the variance of a rice distribution is very close to the variance of a normal distribution with the same sigma (sigma**2), maybe that estimate is close enough for what we're doing.

Bago

@arokem
NIPY developers member
arokem added a note Jun 25, 2012
@arokem
NIPY developers member
arokem added a note Jun 25, 2012
@MrBago
MrBago added a note Jun 25, 2012

We should set up a skype or something to talk about this, It's not clear to me why you're using var(signal) / var(noise). Also it looks like you're implementation of the Gaussian noise scales the noise differently for each voxel. Even though there might be applications where this is useful, it's not physical for MRI. MRI noise generally has the same power over the whole volume.

Also, in the MRI literature there are two ways that noise is usually measured, either the mean signal to noise of diffusion weighted images or the mean signal to noise of b0(no diffusion weighting) images. I believe that the first is a more useful number, but because the SNR of dwi images depends on b-value some people report the second. I believe we should keep that in mind.

@arokem
NIPY developers member
arokem added a note Jun 25, 2012
@MrBago
MrBago added a note Jun 26, 2012
@arokem
NIPY developers member
arokem added a note Jun 26, 2012
@MrBago
MrBago added a note Jun 26, 2012

Ya, the std(rician) isn't exactly sigma, it's pretty close to sigma, especially as sigma gets small (snr gets big). The exact form is on Wikipedia.

@arokem
NIPY developers member
arokem added a note Jun 26, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@arokem
NIPY developers member
arokem commented Jun 26, 2012

Or maybe a factor sqrt(7/3) ?!

@arokem
NIPY developers member
arokem commented Jun 29, 2012

Any more comments here? Otherwise, I will go ahead and merge this some time during the weekend

@MrBago
MrBago commented Jun 29, 2012

Thanks for the hard work ariel, I wanted to give this a little more thought. I think we should be able to solve for the scaling analytically instead of approximating it iteratively, I'm also wondering if we should be using rms(noise) instead of std(noise). I just need to set aside a little time to think about it. If you want, we can merge it and I can propose changes when I have time.

@arokem
NIPY developers member
@arokem
NIPY developers member
arokem commented Jul 12, 2012

I have thought a bit more about the calculation of SNR for DWI: it doesn't really make sense to calculate what is the equivalent of the tSNR that people calculate for fMRI (which is what we have here right now), because part of the variance in the signal is actually what we are interested in. An interesting idea that I got from @rfdougherty is to use the b0 measurements to calculate the noise and to use the Stejskal/Tanner equation to then predict what the signal in each voxel should be like based on the b0 measurement, the b value used in the measurement of the diffusion-weighted images and the mean-diffusivity in each voxel, calculated using DTI from the data.

Essentially, the calculating would like something like this:

sig = np.mean(data[..., b0_idx], -1)
noise = np.var(data[..., bo_idx], -1)
snr = (sig/noise) * np.exp( -b * mean_diffusivity)

Does that make sense to you all?

@arokem
NIPY developers member
arokem commented Jul 24, 2012

Refactor plan:

  1. Specify sigma instead of the SNR for the noise-generating function itself.

  2. Wrap that function in another API that can get a gradient table as its input and use the SNR relative to the b0.

    For the SNR: use the RMS of the noise, which is not mean-subtracted (in contrast to the variance).

    So: SNR = mean(true signal)/RMS(true added noise).

    For Gaussian, this will be equivalent to taking the std, but for Rician it might be different.

    As an alternative, consider setting SNR to be: mean/sigma

  3. To apply SNR estimates from actual data, we might want to be able to estimate the parameters of the Rician distribution from lower order moments of the distribution of b0, or (probably better), use an ROI where the Rician is approximately equal to the Gaussian, so that you can use std.

@travisbot

This pull request fails (merged ce322ef6 into 59eab5c).

@travisbot

This pull request fails (merged 560508d2 into 898a401).

@travisbot

This pull request fails (merged 65419341 into 4ff1bd1).

@travisbot

This pull request fails (merged 969efcb5 into 38c1ec1).

@travisbot

This pull request fails (merged 8f3048a6 into 38c1ec1).

@travisbot

This pull request fails (merged 6fa78d2e into 38c1ec1).

@travisbot

This pull request fails (merged 6fa78d2e into 38c1ec1).

@arokem
NIPY developers member
arokem commented Aug 12, 2012

OK - I think that this should fulfill what we talked about a few weeks ago. @MrBago - what do you think?

@travisbot

This pull request fails (merged ca3f0ef1 into 38c1ec1).

@travisbot

This pull request fails (merged ca3f0ef1 into 38c1ec1).

@travisbot

This pull request passes (merged 80fd4bf3 into dc8b7fd).

@stefanv stefanv commented on an outdated diff Aug 17, 2012
dipy/sims/phantom.py
@@ -62,11 +64,12 @@ def orbital_phantom(bvals=None,
angles and radii define the total thickness options
S0 : double, simulated signal without diffusion gradients applied
Default 100.
- snr : signal to noise ratio
- Used for applying rician noise to the data.
- Default 200. Common is 20.
- background_noise : boolean, Default False
-
+ snr : float, optional
+ The signal to noise ratio sed to apply Rician noise to the data.
@stefanv
stefanv added a note Aug 17, 2012

sed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stefanv stefanv commented on an outdated diff Aug 17, 2012
dipy/sims/phantom.py
+ Gudbjartsson, H and Patz, S (2008). The Rician Distribution of Noisy MRI
@stefanv
stefanv added a note Aug 17, 2012
References
---------------
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stefanv stefanv commented on an outdated diff Aug 18, 2012
dipy/sims/phantom.py
@@ -74,7 +77,16 @@ def orbital_phantom(bvals=None,
Notes
--------
Crossings can be created by adding multiple orbitual_phantom outputs.
+
+ In these simulations, we can ask for Rician noise to be added. In
+ that case, the definition of SNR is as follows:
+
+ SNR = mean(true signal)/RMS(true added noise).
@stefanv
stefanv added a note Aug 18, 2012

Std = RMS only under assumption of zero-mean noise. Is this the case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stefanv stefanv commented on an outdated diff Aug 18, 2012
dipy/sims/phantom.py
@@ -74,7 +77,16 @@ def orbital_phantom(bvals=None,
Notes
--------
Crossings can be created by adding multiple orbitual_phantom outputs.
@stefanv
stefanv added a note Aug 18, 2012

orbitual -> orbital

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stefanv stefanv commented on an outdated diff Aug 18, 2012
dipy/sims/phantom.py
@@ -84,7 +96,7 @@ def f(t):
z=np.linspace(-1,1,len(x))
return x,y,z
- data=orbitual_phantom(func=f)
+ data=orbital_phantom(func=f)
@stefanv
stefanv added a note Aug 18, 2012

data = orbital_phantom(func=f)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stefanv stefanv commented on an outdated diff Aug 18, 2012
dipy/sims/phantom.py
- return np.sqrt(K/np.float(snr))*np.random.randn(*vol.shape)
- noise1=gaussian_noise(vol)
- noise2=gaussian_noise(vol)
- return np.sqrt((vol+noise1)**2+noise2**2)
-
-def add_gaussian_noise(vol,snr=20):
- """ add gaussian noise in a 4D array with a specific snr
-
- Parameters
- -----------
- vol : array, shape (X,Y,Z,W)
- snr : float,
- signal to noise ratio
-
+
+ sigma: float
@stefanv
stefanv added a note Aug 18, 2012

sigma : float, and elsewhere

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stefanv stefanv commented on an outdated diff Aug 18, 2012
dipy/sims/phantom.py
@@ -131,76 +143,127 @@ def f(t):
#FA=ten.fa()
#FA[np.isnan(FA)]=0
#vol[np.isnan(vol)]=0
+
+ if snr is not None:
+ # We start by guessing that sigma should be approximately such that the
+ # snr is right and using: snr = mean_sig/rms => rms = mean_sig/snr
+ mean_sig = np.mean(vol)
+ sigma = mean_sig/snr
+ vol_w_noise = add_noise(vol, sigma, noise_type='rician')
+ noise = vol - vol_w_noise
@stefanv
stefanv added a note Aug 18, 2012

No gripes with this PR, so I'll just comment in general on the dangers of floating point subtraction and losing precision:

s = np.ones((10))*1e10; n = np.random.random((10)) * 1e-5; sn = s+n
np.mean(sn - s), np.mean(n)

However, since this situation would only occur if you have very small noise levels, it really doesn't apply here :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@MrBago MrBago commented on an outdated diff Aug 18, 2012
dipy/sims/phantom.py
@@ -131,76 +143,127 @@ def f(t):
#FA=ten.fa()
#FA[np.isnan(FA)]=0
#vol[np.isnan(vol)]=0
+
+ if snr is not None:
+ # We start by guessing that sigma should be approximately such that the
+ # snr is right and using: snr = mean_sig/rms => rms = mean_sig/snr
+ mean_sig = np.mean(vol)
+ sigma = mean_sig/snr
+ vol_w_noise = add_noise(vol, sigma, noise_type='rician')
@MrBago
MrBago added a note Aug 18, 2012

We can replace this with vol_w_noise = add_noise(vol, sigma, noise_type='rician', scale=1./scipy.stats.rice(snr).std())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@MrBago MrBago commented on an outdated diff Aug 18, 2012
dipy/sims/phantom.py
@@ -131,76 +143,127 @@ def f(t):
#FA=ten.fa()
#FA[np.isnan(FA)]=0
#vol[np.isnan(vol)]=0
+
+ if snr is not None:
+ # We start by guessing that sigma should be approximately such that the
+ # snr is right and using: snr = mean_sig/rms => rms = mean_sig/snr
+ mean_sig = np.mean(vol)
+ sigma = mean_sig/snr
@MrBago
MrBago added a note Aug 18, 2012

Sorry, I don't know what I'm smoking.

I think in order to avoid the while loop we can replace sigma = mean_sig/snr with sigma = mean_sig / snr / scipy.stats.rice(snr).std(). The only problem is that scipy.stats.rice wiggs out if snr == 0 or > 37.5 so we need to do something like:

if snr > 37.5:
    scale = 1
elif snr == 0:
    scale = np.sqrt((4 - np.pi)/2)
else:
    scale = scipy.stats.rice(snr).std()

sigma = mean_sig / snr / scale

I'm kinda tiered so someone else should have a second look at that and make sure it makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@travisbot

This pull request fails (merged 552a13c3 into dc8b7fd).

@arokem
NIPY developers member
arokem commented Aug 18, 2012

@MrBago: is this what you meant? As travisbot was quick to point out, test_snr now fails with this. First, do you think the test is OK? Second, I debugged on this and for the cases used in the test, the scale variable is very close to 1, so this block of code doesn't really do much. That is, you end up with a sigma very similar to the one you would have gotten from mean_sig/snr. Unfortunately, for the Rician case, this is pretty far off from the sigma you actually want. Note that sigma is not the standard deviation of the Rician, but is a bit off of that (because of what the code in add_noise does with 'sigma').

@arokem
NIPY developers member
arokem commented Nov 10, 2012

@MrBago - does this make sense to you? We had to add a 'fudge factor' of sqrt(2) to make the SNR numbers come out right here (that last commit). But why sqrt(2)? Any ideas?

@coryahrens

sounds like the length of a unit complex number...

@MrBago
MrBago commented Nov 10, 2012

You have sqrt(x / 2) * sqrt(2) which is sqrt(x) so I'm not sure what you're doing here.

@MrBago
MrBago commented Nov 10, 2012

I'm apprehensive about the way this function is set up. MRI noise is poorly described by "std" or "variance", and will have a different variance in different parts of the image and in different images, (ie different b-values in a dwi acquisition). The sigma is a much better description and should be relatively consistent, at least within one image (and hopefully even at different b-value acquisitions).

@arokem
NIPY developers member
@arokem
NIPY developers member
@MrBago
MrBago commented Nov 10, 2012

The problem is that you're thinking of noise as (signal + noise). In MRI measured signal is abs(signal + complex_noise). This means that var(measured - signal) does not have a consistent distribution independent of signal. The fudge factor comes from assuming (signal == 0), which you can do outside the brain.

@MrBago
MrBago commented Nov 10, 2012

Calculating a different sigma in each voxel is unphysical, it's an interesting exercise, but MRI scanners will generate noise with a consistent sigma, and an inconsistent variance or "SNR". If you don't believe me we should set up a phantom simulation.

@arokem
NIPY developers member
@MrBago
MrBago commented Nov 10, 2012

From Maxime Descoteaux's 2007 qball paper

The noise is
generated with a complex Gaussian noise with a standard
deviation of σ, producing a S0 signal with SNR = 1/σ , that
is we define SNR as the ratio of maximum signal intensity
of S0 to the standard deviation σ of the complex Gaussian
noise.

ie noise is defined as b0-signal / sigma.

@MrBago
MrBago commented Nov 10, 2012

Sorry I mean SNR is defined as b0-signal / sigma.

@arokem
NIPY developers member
@arokem
NIPY developers member
@arokem
NIPY developers member
@MrBago
@arokem
NIPY developers member
@MrBago
@arokem
NIPY developers member
@arokem arokem RF: Moved noise-addition functions to the voxel sub-module.
Also: adopted the convention used by Descoteaux et al. to define SNR.
e7200c9
@coryahrens

Could you please explain why MRI images are not well characterized by mean and variance. My understanding of Rician noise was that it covers MRI pretty well. An unusual feature of this noise is that it is non-stationary, i.e., the moments (mean, variance, etc) are not constant; they change with the value of the signal.

Isn't the standard deviation of the Gaussian noise for each channel fairly constant over the whole image? In this case, the sigma in the Rician distribution would be constant, but the signal value would be changing and hence the standard deviation of the whole Rician distribution changes over the volume.

@MrBago
MrBago commented Nov 10, 2012

That's a good way of describing it. I just meant that because the Rician distribution is non-stationary, we expect variance(signal) to vary over a volume, even though variance(complex_noise) is pretty consistent over the volume.

@arokem
NIPY developers member
arokem commented Nov 30, 2012

OK @Garyfallidis, this is ready for you to take a look. In particular, there's still one test failing for the orbital phantom. We're not getting the FA you would expect for the single tensor voxel. Any ideas?

@arokem
NIPY developers member
arokem commented Nov 30, 2012

Maybe this is because there is no single-tensor voxel anywhere in the volume?

@Garyfallidis
NIPY developers member

Okay Travis reports more than one errors here.

1077FAIL: Doctest: dipy.sims.phantom.orbital_phantom
1078----------------------------------------------------------------------
1079Traceback (most recent call last):
1080 File "/usr/lib/python2.7/doctest.py", line 2201, in runTest
1081 raise self.failureException(self.format_failure(new.getvalue()))
1082AssertionError: Failed doctest test for dipy.sims.phantom.orbital_phantom
1083 File "/usr/local/lib/python2.7/dist-packages/dipy/sims/phantom.py", line 92, in orbital_phantom
1084
1085----------------------------------------------------------------------
1086File "/usr/local/lib/python2.7/dist-packages/dipy/sims/phantom.py", line 142, in dipy.sims.phantom.orbital_phantom
1087Failed example:
1088 data = orbital_phantom(func=f)
1089Exception raised:
1090 Traceback (most recent call last):
1091 File "/usr/lib/python2.7/doctest.py", line 1289, in __run
1092 compileflags, 1) in test.globs
1093 File "", line 1, in
1094 data = orbital_phantom(func=f)
1095 File "/usr/local/lib/python2.7/dist-packages/dipy/sims/phantom.py", line 146, in orbital_phantom
1096 if gtab.bvals is None:
1097 AttributeError: 'NoneType' object has no attribute 'bvals'

In phantom.py line 146. if gtab.bvals is None should be if gtab is None.

1035ERROR: dipy.reconst.tests.test_dti.test_TensorModel
1036----------------------------------------------------------------------
1037Traceback (most recent call last):
1038 File "/usr/lib/python2.7/dist-packages/nose/case.py", line 197, in runTest
1039 self.test(*self.arg)
1040 File "/usr/local/lib/python2.7/dist-packages/dipy/reconst/tests/test_dti.py", line 22, in test_TensorModel
1041 assert_equal(dtifit.fa < 0.5, True)
1042 File "/usr/local/lib/python2.7/dist-packages/dipy/core/onetime.py", line 175, in get
1043 val = self.getter(obj)
1044 File "/usr/local/lib/python2.7/dist-packages/dipy/reconst/dti.py", line 143, in fa
1045 all_zero = (np.isclose(ev1, 0) & np.isclose(ev2, 0) & np.isclose(ev3, 0))
1046AttributeError: 'module' object has no attribute 'isclose'

Which version of numpy introduces isclose? It is not supported in 1.6.1 which at least I use.

1100======================================================================
1101FAIL: Doctest: dipy.sims.voxel.add_noise
1102----------------------------------------------------------------------
1103Traceback (most recent call last):
1104 File "/usr/lib/python2.7/doctest.py", line 2201, in runTest
1105 raise self.failureException(self.format_failure(new.getvalue()))
1106AssertionError: Failed doctest test for dipy.sims.voxel.add_noise
1107 File "/usr/local/lib/python2.7/dist-packages/dipy/sims/voxel.py", line 48, in add_noise
1108
1109----------------------------------------------------------------------
1110File "/usr/local/lib/python2.7/dist-packages/dipy/sims/voxel.py", line 86, in dipy.sims.voxel.add_noise
1111Failed example:
1112 signal_w_noise = add_noise(signal, snr=10, noise_type='rician')
1113Exception raised:
1114 Traceback (most recent call last):
1115 File "/usr/lib/python2.7/doctest.py", line 1289, in __run
1116 compileflags, 1) in test.globs
1117 File "", line 1, in
1118 signal_w_noise = add_noise(signal, snr=10, noise_type='rician')
1119 TypeError: add_noise() takes at least 3 arguments (3 given)
1120
1121
1122----------------------------------------------------------------------

You forgot to set S0. I think?

Finally, concerning the error FA which is different. Is the FA value very different?

@matthew-brett
NIPY developers member
@arokem
NIPY developers member
arokem commented Nov 30, 2012

OK - we are down to just the one I meant. Strangely, while travis is getting:

ACTUAL: 1000.0
DESIRED: 686

I am getting on my machine:

ACTUAL: 979.0
DESIRED: 686

In both cases, the actual resulting FA is larger than expected from the calculation, based on the input axial and radial diffusivities.

@stefanv
stefanv commented Nov 30, 2012

Sorry, isclose was only added in NumPy 1.7.0.

@MrBago
MrBago commented Dec 1, 2012

@arokem, can you check to see if the signal being passed to TensorModel.fit if < 1. I believe that TensorModel sets all signals < min_signal to min_signal (so that we can take the log). You can try setting min_signal to something very small, like 1e-8 and see if that gives the right answer.

@arokem
NIPY developers member
arokem commented Dec 1, 2012
@arokem
NIPY developers member
arokem commented Dec 1, 2012
@Garyfallidis
NIPY developers member

@arokem Do you mean S0 to 100 rather than 0?

I am getting on my machine:

ACTUAL: 812.0
DESIRED: 686

Indeed the problem seems to take place in regions outside the fiber in the phantom. What I suppose should be the thresholded or masked region?

How do you set the mask using TensorModel? In multi_voxel.py the mask is a parameter at the model.fit(data, mask).
I don't see how this is available in TensorModel but I see it available in the old Tensor class.

Apart from that we need also to understand why the max is different from one computer to the other.
GGT!

@arokem arokem BF: Small eigenvalues => FA == 0
Make sure that when all eigenvalues of a tensor are small, they are effectively
set to 0, so that the FA doesn't misbehave with really small values of evals.

This also sets straight the tests for the orbital_phantom.
5d773c2
@arokem
NIPY developers member
arokem commented Dec 5, 2012

Can we just merge this already?

@Garyfallidis
NIPY developers member

Yes.

@Garyfallidis Garyfallidis merged commit 6a69476 into nipy:master Dec 5, 2012

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.