Add gaussian and poisson noise options to projection applications#860
Conversation
axel-grc
force-pushed
the
Noise
branch
2 times, most recently
from
b2d59a8 to
ef55545
Compare
axel-grc
force-pushed
the
Noise
branch
4 times, most recently
from
1cbeb80 to
f0b3a98
Compare
lesaintjerome
left a comment
lesaintjerome
left a comment
There was a problem hiding this comment.
I did a few tests. It seems to run fine.
The noise feature in rtkdrawgeometricphantom and rtkprojectshepplogan has not been removed. Should it be?
| section "Gaussian noise" | ||
| option "gaussian" - "Gaussian noise parameters: <mean> Noise level and <std> Noise standard deviation" double multiple no | ||
|
|
||
| section "Poisson noise" | ||
| option "poisson" - "Poisson noise parameters: <I0> Number of impinging photons per pixel and <muref> reference linear attenuation coefficient" double multiple no |
There was a problem hiding this comment.
This seems to make the mu_ref parameter mandatory. Why not. Though we could mention something like "e.g. set 0.01879, which is the attenuation coefficient of water at 75 keV".
There was a problem hiding this comment.
Same could be said about the mean parameter for gaussian noise. Default=0. would make sense, to me.
There was a problem hiding this comment.
The problem is that we can't use different default values for lists with gengetopt, so the same default value will be used for mean and std for example.
We can maybe add theses default values to the implementation part ?
There was a problem hiding this comment.
Or have slightly more complex Gaussian and Poisson ggo sections, with 2 parameters each?
axel-grc
changed the title
SimonRit
force-pushed
the
Noise
branch
3 times, most recently
from
ad34e3c to
48182a1
Compare
SimonRit
left a comment
SimonRit
left a comment
There was a problem hiding this comment.
I have modified this PR to make it to my taste. Main changes are:
- muref now has a default
- mean of the gaussian has been removed
- gaussian is applied both with Poisson (before the log) and without (without any exp / log)
|
@axel-grc and @lesaintjerome, do you mind re-reviewing? And testing the Python version @axel-grc if you don't mind. |
| rtknoise_group.add_argument( | ||
| "--gaussian", | ||
| help="Additive Gaussian noise standard deviation (pre-log if --poisson given, without exp/log otherwise)", | ||
| type=float, | ||
| ) | ||
|
|
There was a problem hiding this comment.
In my small tests, what I had in mind was: take Poisson noise, then add 10% gaussian noise. So I needed to estimate the pre-log values and set the std accordingly).
We could instead pass a "noise level" (e.g. 10%) and the std would be computed as the pre_log_mean * noise_level.
But it may conflict with other use cases where the user knows exactly the level of noise he wants to simulate.
There was a problem hiding this comment.
I find it a bit too specific. Why pre_log_mean * noise_level? Gaussian noise combined with Poisson noise is mainly here to simulate electronic noise no? So it should be independent of the number of photons emitted? I'm in favor of leaving it as is.
There was a problem hiding this comment.
Sure. You're right ! Forget this comment.
| rtknoise_group = parser.add_argument_group("Poisson and Gaussian noise") | ||
| rtknoise_group.add_argument( | ||
| "--poisson", | ||
| help="Comma-separated pre-log Poisson noise parameters: I0 pre-log normalization and (optional) pre-exp multiplicative factor (default 0.01879)", |
There was a problem hiding this comment.
Should we say what does 0.01879 correspond to?
|
Did some quick tests. It seems to work fine. |
Co-authored-by: Simon Rit <simon.rit@creatis.insa-lyon.fr>
|
Looks good to me, the only downside is that with poisson noise arguments, the first comma separated value has a default, but not the second, so we need to enter the two values, even if we want the default for the first. |
3 participants
No description provided.