Skip to content
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 cta_simulation notebook, use CTA prod 3 IRFs #2000

Merged
merged 2 commits into from Jan 24, 2019

Conversation

@registerrier
Copy link
Contributor

@registerrier registerrier commented Jan 23, 2019

This PR adapt to the new SensitivityEstimator API introduced in #1998 . It provides an approach to estimate the sensitivity with the full enclosure IRFs distributed with the CTA 1DC and shipped in gammapy datasets. This solves issues #1824 and #1943 .

@registerrier registerrier added this to the 0.10 milestone Jan 23, 2019
@registerrier registerrier self-assigned this Jan 23, 2019
@registerrier registerrier requested review from Bultako and adonath Jan 23, 2019
@registerrier registerrier changed the title Corrected cat_simulation notebook working with prod 3 IRFs Corrected cta_simulation notebook working with prod 3 IRFs Jan 23, 2019
Copy link
Member

@adonath adonath left a comment

Thanks @registerrier! This looks good to me, it's basically ready to merge as is.

Here are a few (optional) minor suggestions for improvement:

Maybe you could add one short sentence explaining the difference between the gamma and the significance criterion (this was not clear to me at a first read, I had to look into the code...). You could show a little more information, such as plotting the on-region radius against energy or show the background spectrum.

@registerrier
Copy link
Contributor Author

@registerrier registerrier commented Jan 24, 2019

Thanks @adonath . Good points. I have added some explanations on parameter usage as well as a more explicit creation of SensitivityEstimator.
A control plot showing background counts as well as ON region radius with energy has been added.

@registerrier registerrier merged commit cc073e1 into gammapy:master Jan 24, 2019
2 of 4 checks passed
2 of 4 checks passed
Scrutinizer Created
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
gammapy.gammapy Build #20190124.3 succeeded
Details
@cdeil cdeil changed the title Corrected cta_simulation notebook working with prod 3 IRFs Fix cta_simulation notebook, use CTA prod 3 IRFs Jan 25, 2019
@registerrier registerrier deleted the registerrier:cta_sensitivity branch Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants