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

Generalise exponential cutoff power law spectral model #2574

Merged
merged 8 commits into from Nov 22, 2019

Conversation

@bkhelifi
Copy link
Contributor

bkhelifi commented Nov 20, 2019

Description
This PR contains the generalisation of the power law with an exponential cut-off, adding a power inside the exponential term.

Dear reviewer
The model has been improved by adding an 'alpha' parameter, being egual to 1 by default. Its peak function has been adapted accordingly, as an analytical function exists (its formula has been re-checked using plots).
In parallel, the corresponding test function has been updated to test the computation with an alpha different from 1.

@bkhelifi bkhelifi added the effort-low label Nov 20, 2019
@bkhelifi bkhelifi added this to the 0.15 milestone Nov 20, 2019
@bkhelifi bkhelifi requested a review from adonath Nov 20, 2019
@bkhelifi bkhelifi added this to To do in gammapy.spectrum via automation Nov 20, 2019
@adonath adonath added feature and removed effort-low labels Nov 20, 2019
Copy link
Contributor

registerrier left a comment

See inline comments.
There are a number of fails.

@registerrier

This comment has been minimized.

Copy link
Contributor

registerrier commented Nov 21, 2019

There is a remaining issue in the analysis_3d.ipynb notebook. This is due to the size of the covariance matrix which has changed.
I think the operation required is ugly and error prone. I don't know if we can avoid it.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Nov 21, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2574      +/-   ##
==========================================
- Coverage    91.5%   91.49%   -0.01%     
==========================================
  Files         140      140              
  Lines       15970    15994      +24     
==========================================
+ Hits        14613    14634      +21     
- Misses       1357     1360       +3
Impacted Files Coverage Δ
gammapy/modeling/models/spectral.py 98% <100%> (ø) ⬆️
gammapy/time/lightcurve_estimator.py 94.93% <0%> (-1.27%) ⬇️
gammapy/spectrum/flux_point.py 91.19% <0%> (-0.35%) ⬇️
gammapy/modeling/serialize.py 98.85% <0%> (-0.02%) ⬇️
gammapy/time/variability.py 100% <0%> (ø) ⬆️
gammapy/spectrum/sensitivity.py 100% <0%> (ø) ⬆️
gammapy/cube/edisp_map.py 95.45% <0%> (ø) ⬆️
gammapy/irf/psf_3d.py 98.17% <0%> (ø) ⬆️
gammapy/cube/fit.py 89.7% <0%> (ø) ⬆️
gammapy/cube/psf_map.py 95.76% <0%> (ø) ⬆️
... and 6 more

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 8fc61f6...4495600. Read the comment docs.

@bkhelifi bkhelifi force-pushed the bkhelifi:SuperExponentialPL branch from 1e2f0b1 to f46af0e Nov 22, 2019
Bruno Khelifi
Copy link
Contributor

registerrier left a comment

Thanks @bkhelifi ! That's good to go.

@registerrier registerrier merged commit a2adc29 into gammapy:master Nov 22, 2019
12 checks passed
12 checks passed
greeting
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Scrutinizer Analysis: No new issues – Tests: passed
Details
codecov/patch 100% of diff hit (target 91.5%)
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +8.49% compared to 8fc61f6
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
gammapy.gammapy #20191122.8 succeeded
Details
gammapy.gammapy (DevDocs) DevDocs succeeded
Details
gammapy.gammapy (Lint) Lint succeeded
Details
gammapy.gammapy (Test Python36) Test Python36 succeeded
Details
gammapy.gammapy (Test Windows36) Test Windows36 succeeded
Details
gammapy.gammapy (Test Windows37) Test Windows37 succeeded
Details
gammapy.spectrum automation moved this from To do to Done Nov 22, 2019
@cdeil cdeil changed the title Generalisation of the power law with an exponential cut-off Generalise exponential cutoff power law spectral model Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4 participants
You can’t perform that action at this time.