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

Fix Wstat #789

Merged
merged 1 commit into from Nov 21, 2016

Conversation

Projects
None yet
3 participants
@joleroi
Contributor

joleroi commented Nov 21, 2016

@joleroi joleroi added the bug label Nov 21, 2016

@joleroi joleroi added this to the 0.5 milestone Nov 21, 2016

@joleroi joleroi self-assigned this Nov 21, 2016

@joleroi

This comment has been minimized.

Show comment
Hide comment
@joleroi
Contributor

joleroi commented Nov 21, 2016

@joleroi joleroi merged commit 1481526 into gammapy:master Nov 21, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@joleroi joleroi deleted the joleroi:fix_wstat branch Nov 21, 2016

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Nov 22, 2016

Member

@joleroi - Thanks!

@registerrier - Could you please review the WSTAT values for n_on=0 and n_off=0 cases in the table at the end of this section?
http://docs.gammapy.org/en/latest/stats/fit_statistics.html#poisson-data-with-background-measurement
Is that correct?

@joleroi and @registerrier - Are Gammapy WSTAT results now consistent with XSPEC or Sherpa results? I think it would be good to know if that's the case, and either way add a note to the Gammapy docs page?

Member

cdeil commented Nov 22, 2016

@joleroi - Thanks!

@registerrier - Could you please review the WSTAT values for n_on=0 and n_off=0 cases in the table at the end of this section?
http://docs.gammapy.org/en/latest/stats/fit_statistics.html#poisson-data-with-background-measurement
Is that correct?

@joleroi and @registerrier - Are Gammapy WSTAT results now consistent with XSPEC or Sherpa results? I think it would be good to know if that's the case, and either way add a note to the Gammapy docs page?

@joleroi

This comment has been minimized.

Show comment
Hide comment
@joleroi

joleroi Nov 22, 2016

Contributor

@joleroi and @registerrier - Are Gammapy WSTAT results now consistent with XSPEC or Sherpa results?

Yes they agree with Sherpa

see https://github.com/gammapy/gammapy/blob/master/dev/sherpa/stats/compare_wstat.py
and https://github.com/gammapy/gammapy/blob/master/gammapy/stats/tests/test_fit_statistics.py

For XSPEC we don't know, because we only have the implementation you added to dev

https://github.com/gammapy/gammapy/blob/master/dev/sherpa/stats/xspec_stats.py

and we don't know if it's correct or not 😉 (but for now it looks fine)

Contributor

joleroi commented Nov 22, 2016

@joleroi and @registerrier - Are Gammapy WSTAT results now consistent with XSPEC or Sherpa results?

Yes they agree with Sherpa

see https://github.com/gammapy/gammapy/blob/master/dev/sherpa/stats/compare_wstat.py
and https://github.com/gammapy/gammapy/blob/master/gammapy/stats/tests/test_fit_statistics.py

For XSPEC we don't know, because we only have the implementation you added to dev

https://github.com/gammapy/gammapy/blob/master/dev/sherpa/stats/xspec_stats.py

and we don't know if it's correct or not 😉 (but for now it looks fine)

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Nov 22, 2016

Member

@registerrier - Do you know how to run XSPEC directly to check? I guess people are still commonly using it, and it is relevant to know if Gammapy / Sherpa is the same or not, right?

Member

cdeil commented Nov 22, 2016

@registerrier - Do you know how to run XSPEC directly to check? I guess people are still commonly using it, and it is relevant to know if Gammapy / Sherpa is the same or not, right?

@registerrier

This comment has been minimized.

Show comment
Hide comment
@registerrier

registerrier Nov 22, 2016

Contributor

I am not sure the tests described above can be done in an easy way with XSPEC.

I would say that as long as we understand what we put in and we have agreement with sherpa, we are fine. No?

Contributor

registerrier commented Nov 22, 2016

I am not sure the tests described above can be done in an easy way with XSPEC.

I would say that as long as we understand what we put in and we have agreement with sherpa, we are fine. No?

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Nov 23, 2016

Member

@registerrier - My understanding is that Sherpa WSTAT is new and not very well tested / documented (see e.g. sherpa/sherpa#314).
So a comparison with XSPEC or other codes, and a derivation of our formul, especially for the WSTAT for bins with n_on=0 or n_off=0, would be good.

But yes, if it's not easy to do with XSPEC, and our results are consistent with Sherpa and based on well-checked formulas, it's good enough. @registerrier - I was mainly pinging you to review the formulas and description in this docs section one last time (before WSTAT is ready for 1.0 and publication):
http://docs.gammapy.org/en/latest/stats/fit_statistics.html#poisson-data-with-background-measurement

Member

cdeil commented Nov 23, 2016

@registerrier - My understanding is that Sherpa WSTAT is new and not very well tested / documented (see e.g. sherpa/sherpa#314).
So a comparison with XSPEC or other codes, and a derivation of our formul, especially for the WSTAT for bins with n_on=0 or n_off=0, would be good.

But yes, if it's not easy to do with XSPEC, and our results are consistent with Sherpa and based on well-checked formulas, it's good enough. @registerrier - I was mainly pinging you to review the formulas and description in this docs section one last time (before WSTAT is ready for 1.0 and publication):
http://docs.gammapy.org/en/latest/stats/fit_statistics.html#poisson-data-with-background-measurement

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