LM-SAL / aiapy

Python library for AIA data analysis
https://aiapy.readthedocs.io/en/stable/
BSD 3-Clause "New" or "Revised" License
5 stars 3 forks source link

Function for estimating uncertainty on intensities - [merged] #240

Closed nabobalis closed 8 months ago

nabobalis commented 3 years ago

In GitLab by @wtbarnes on Jun 11, 2021, 14:21

Merges calc-errors -> master

Fixes #60

A few things left to do:

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 23, 2021, 07:17

added 4 commits

Compare with previous version

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 29, 2021, 10:56

marked the checklist item Clean up docstrings as completed

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 29, 2021, 10:56

marked the checklist item Naming? estimate_uncertainty or maybe just uncertainty? as completed

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 29, 2021, 12:29

added 2 commits

Compare with previous version

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 29, 2021, 12:31

I've decided to defer the example to another merge request. Additionally, I'm not sure we need to handle the single dispatch stuff now. There are probably a number of places in the repo where it would be nice to have the ability to operate both on scalars/arrays as well as maps.

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 29, 2021, 12:33

added 6 commits

Compare with previous version

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 29, 2021, 12:46

added 1 commit

Compare with previous version

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 29, 2021, 12:46

marked the checklist item changelog as completed

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 29, 2021, 12:46

marked this merge request as ready

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 29, 2021, 12:47

Regarding the units on the shot noise issue, @pboerner gave the following response a while back:

The units on the shot noise are confusing…it looks like you’ve cancelled all the units to get the number of photons, taken the sqrt, and put the units back on, which is basically what the IDL function does (it doesn’t have units of course). I guess you could be really rigorous and calculate the standard deviation of the Poisson distribution; it only really differs from sqrt(n) for small n, but maybe that’s more trouble than it’s worth…?

The current implementation is still a bit ambiguous, but I don't think it needs to block this merge request.

nabobalis commented 2 years ago
        The corresponding channel for the observed counts.
nabobalis commented 2 years ago
nabobalis commented 2 years ago

Even if it's kind of redundant, a description here would be nice.

nabobalis commented 2 years ago
        Use the preflight photometric calibration. If True, ``include_eve`` must be False.
nabobalis commented 2 years ago
        Use the EVE photometric calibration. If True, ``include_preflight`` must be False.
nabobalis commented 2 years ago
        If True, include the atomic data errors from CHIANTI in the uncertainty.
nabobalis commented 2 years ago
    `~astropy.units.Quantity`
nabobalis commented 2 years ago

What is going on?

nabobalis commented 2 years ago

Is a paper or a source for this value?

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 30, 2021, 06:05

Commented on aiapy/calibrate/uncertainty.py line 36

changed this line in version 6 of the diff

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 30, 2021, 06:05

Commented on aiapy/calibrate/tests/test_uncertainty.py line 3

changed this line in version 6 of the diff

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 30, 2021, 06:05

Commented on aiapy/calibrate/uncertainty.py line 41

changed this line in version 6 of the diff

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 30, 2021, 06:05

Commented on aiapy/calibrate/uncertainty.py line 43

changed this line in version 6 of the diff

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 30, 2021, 06:05

Commented on aiapy/calibrate/uncertainty.py line 45

changed this line in version 6 of the diff

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 30, 2021, 06:05

Commented on aiapy/calibrate/uncertainty.py line 54

changed this line in version 6 of the diff

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 30, 2021, 06:05

added 1 commit

Compare with previous version

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 30, 2021, 06:17

added 1 commit

Compare with previous version

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 30, 2021, 06:18

Commented on aiapy/calibrate/uncertainty.py line 73

This text comes from @pboerner. I'm not sure if there is a reference anywhere?

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Sep 30, 2021, 06:30

Commented on aiapy/calibrate/uncertainty.py line 65

:shrug:

This is the IDL code,

numphot = counts / dnperpht
stdevphot = SQRT(numphot)
shotnoise = (stdevphot * dnperpht)
shotnoise = nullcount + shotnoise / SQRT(n_sample)      ;   DN uncertainty due to shot noise

shotnoise should have the same units as counts, DN/pixel. dnperpht has units of DN / photon so stddevphot has units of sqrt(photon / pixel) such that shotnoise has units of DN / sqrt(photon pixel).

Maybe the trick here is that photon and pixel are really "unitless" so as long as we wind up with DN in the numerator, it doesn't really matter?

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Oct 19, 2021, 12:05

Commented on aiapy/calibrate/uncertainty.py line 65

changed this line in version 8 of the diff

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Oct 19, 2021, 12:05

added 1 commit

Compare with previous version

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Oct 19, 2021, 12:06

marked the checklist item Clarify units in shot noise calculation as completed

nabobalis commented 2 years ago

In GitLab by @codecov on Oct 19, 2021, 12:36

Codecov Report

Merging #116 (9508db3) into master (9c08605) will increase coverage by 0.14%. The diff coverage is 98.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
+ Coverage   97.26%   97.41%   +0.14%     
==========================================
  Files          15       16       +1     
  Lines         475      541      +66     
==========================================
+ Hits          462      527      +65     
- Misses         13       14       +1     
Impacted Files Coverage Δ
aiapy/calibrate/uncertainty.py 97.36% <97.36%> (ø)
aiapy/calibrate/__init__.py 100.00% <100.00%> (ø)
aiapy/calibrate/prep.py 98.18% <100.00%> (ø)
aiapy/calibrate/util.py 100.00% <100.00%> (ø)
aiapy/response/channel.py 100.00% <100.00%> (ø)
aiapy/util/util.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9c08605...9508db3. Read the comment docs.

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Oct 19, 2021, 17:23

added 1 commit

Compare with previous version

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Oct 19, 2021, 17:38

added 1 commit

Compare with previous version

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Oct 23, 2021, 19:11

added 1 commit

Compare with previous version

nabobalis commented 2 years ago

approved this merge request

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Oct 26, 2021, 12:30

resolved all threads

nabobalis commented 2 years ago

In GitLab by @wtbarnes on Oct 26, 2021, 12:32

mentioned in commit 9d7d4a1e2007626e13f61a3a02904ef772d2819c