Skip to content

Conversation

@ferrigno
Copy link

This is just an extension in case of Poissonian errors

@ferrigno ferrigno added the enhancement New feature or request label Aug 20, 2024
@ferrigno ferrigno requested a review from volodymyrss August 20, 2024 16:12
ogip/spec.py Outdated

if 'STAT_ERR' in f['SPECTRUM'].data.names:
stat_err = f['SPECTRUM'].data['STAT_ERR']
elif 'POISSERR' in f['SPECTRUM'].header and f['SPECTRUM'].header['POISSERR']:
Copy link
Member

@volodymyrss volodymyrss Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
elif 'POISSERR' in f['SPECTRUM'].header and f['SPECTRUM'].header['POISSERR']:
elif f['SPECTRUM'].header.get('POISSERR'):

ogip/spec.py Outdated
elif 'POISSERR' in f['SPECTRUM'].header and f['SPECTRUM'].header['POISSERR']:
if 'COUNTS' in f['SPECTRUM'].data.names:
stat_err = np.sqrt(f['SPECTRUM'].data['COUNTS']) / f['SPECTRUM'].header['EXPOSURE']
elif 'RATE' in f['SPECTRUM'].data.names:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that anticipated in the standard? POISSERR with RATE columns? EXPOSURE might be somehow corrected (e.g. deadtime) and RATE*EXPOSURE can be non-int. Interpretation might be complex.

If it is anticipated, could you cite it in a comment for clarity?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the POISSERR keywork is set, it is meant that uncertainty is the square root of counts. As the output is only using rate, a correct assumption is that RATE=COUNTS/EXPOSURE and STAT_ERR = sqrt(COUNTS)/EXPOSURE

ogip/spec.py Outdated
if 'RATE' in f['SPECTRUM'].data.names:
rate = f['SPECTRUM'].data['RATE']
elif 'COUNTS' in f['SPECTRUM'].data.names:
rate = f['SPECTRUM'].data['COUNTS'] / f['SPECTRUM'].header['EXPOSURE']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Header is asked for EXPOSURE many times making the lines longer than needed.

Maybe:

Suggested change
rate = f['SPECTRUM'].data['COUNTS'] / f['SPECTRUM'].header['EXPOSURE']
rate = f['SPECTRUM'].data['COUNTS'] / exposure

and above

exposure = f['SPECTRUM'].header['EXPOSURE']

?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Member

@volodymyrss volodymyrss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also could you please add a test file?

@volodymyrss
Copy link
Member

Did you manage to make progress on this, @ferrigno ? Do you need any help?

@ferrigno
Copy link
Author

ferrigno commented Sep 5, 2024

I think it is ready now.

@ferrigno ferrigno requested a review from volodymyrss September 5, 2024 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants