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

Add modeling and fitting tutorial notebook #2764

Merged
merged 6 commits into from
Feb 11, 2020
Merged

Conversation

QRemy
Copy link
Contributor

@QRemy QRemy commented Feb 5, 2020

Show the loading of datasets, the model creation (LogParabola), the Fit itself, the call to minos_contours, and then plot the result.

The implementation largely re-use the code from the joint-crab validation:
https://github.com/gammapy/gammapy-benchmarks/blob/master/validation/joint-crab/make.py

  • add extra functionality to plot multiple contours levels
  • add outliers filter for aberrant contour points given by Minuit
  • change the interpolation scheme

@codecov
Copy link

codecov bot commented Feb 5, 2020

Codecov Report

Merging #2764 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2764      +/-   ##
==========================================
- Coverage   92.17%   92.17%   -0.01%     
==========================================
  Files         142      142              
  Lines       16158    16157       -1     
==========================================
- Hits        14894    14893       -1     
  Misses       1264     1264
Impacted Files Coverage Δ
gammapy/modeling/fit.py 93.52% <100%> (-0.05%) ⬇️

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 ccd3314...dbd7a78. Read the comment docs.

Copy link
Member

@adonath adonath left a comment

Choose a reason for hiding this comment

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

Thanks a lot @QRemy. A few general comments first:

  • I think we should write a general modeling / fitting notebook. For this we already have an unfilled notebook (https://docs.gammapy.org/0.16/notebooks/modeling.html). Please move over the content (or just rename...)

  • This general fitting notebook should include how to work with Fit class in general:

    • Show how use different fitting backends
    • Show how to access the covariance information
    • Show how to compute likelihood profiles
    • Show how to compute confidence contours
  • The used dataset does not really matter and can remain the same as you have used now.

  • In general I would simplify the shown content for the contours:

    • Just use one sigma value
    • Normally there shouldn't be any outliers, the outlier filtering is somehow distracting from the actual content, if possible remove
    • Here is a somewhat simpler version of the contour interpolation, seems easier to understand then the cumulative distance approach

@QRemy QRemy changed the title Add confidence contours tutorial notebook Add modeling and fitting tutorial notebook Feb 7, 2020
Copy link
Member

@adonath adonath left a comment

Choose a reason for hiding this comment

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

Thanks a lot @QRemy! I have one minor comment:

The amount of code could be reduced a little bit by using e.g. itertools like so:

from itertools import combinations

for par_1, par_2 in combinations(["alpha", "beta", "amplitude"], r=2):
    contour = fit.minos_contours(par_1, par_2)

As I already mentioned in the dev call this morning, I think it's always a good idea to show users some functionality of the Python standard library.

Copy link
Contributor

@registerrier registerrier left a comment

Choose a reason for hiding this comment

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

Thanks @QRemy . This looks very nice.
As we discussed, at some point some of the contour plotting functions should go in the API itself.
There is one thing that puzzles me. The best beta value is 0.26 but the 1 sigma contours does not seem to include it. Is there a problem, or did I miss something?

Also you might use alpha instead of gamma for the index to be consistent with the parameter naming scheme.

@QRemy
Copy link
Contributor Author

QRemy commented Feb 7, 2020

Thanks @QRemy . This looks very nice.
As we discussed, at some point some of the contour plotting functions should go in the API itself.
There is one thing that puzzles me. The best beta value is 0.26 but the 1 sigma contours does not seem to include it. Is there a problem, or did I miss something?

Also you might use alpha instead of gamma for the index to be consistent with the parameter naming scheme.

The difference in beta seems to comes from the np.log(10) factor here:
contours["contour_alpha_beta"] = { "alpha": contour["x"].tolist(), "beta": (contour["y"]*np.log(10)).tolist(), }

This is copied from the validation code, was it something introduced to match the parametrization of the joint Crab paper ?

@registerrier
Copy link
Contributor

Ah yes indeed. The joint crab paper used a log10 expression to match the sherpa model which is expressed in log10 hence the factor. This should be removed here.

- add  a tutorial notebook showing how to perform the computation of confidence contours for parameters after a fit as suggested in gammapy#2682
 
Show the loading of datasets, the model creation (LogParabola), the Fit itself, the call to minos_contours, and then plot the result.

The implementation largely re-used the code from the joint-crab validation:
 https://github.com/gammapy/gammapy-benchmarks/blob/master/validation/joint-crab/make.py

- add extra functionality to plot multiple contours levels
- add outliers filter for aberrant contour points given by Minuit
- change the interpolation scheme
@adonath adonath added this to the 0.17 milestone Feb 11, 2020
Copy link
Member

@adonath adonath left a comment

Choose a reason for hiding this comment

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

Thanks @QRemy, looks good to me now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants