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 ECPL model, energy flux and integration methods #641

Merged
merged 2 commits into from Jul 22, 2016

Conversation

Projects
None yet
3 participants
@adonath
Member

adonath commented Jul 21, 2016

This PR adds an ECPL model, energy flux and integration methods to SpectralModel.

I've changed the latex formulae to be consistent with the HGPS paper. I.e using \phi for differential, F for integral and G for energy flux.

@joleroi Is there an ECPL model in sherpa that could be used in to_sherpa()?

@@ -34,7 +34,6 @@
'power_law_pivot_energy',
'power_law_df_over_f',
'power_law_flux',
'power_law_energy_flux',

This comment has been minimized.

@cdeil

cdeil Jul 21, 2016

Member

Please don't remove this now. This breaks other parts in Gammapy and other scripts:
https://ci.appveyor.com/project/cdeil/gammapy/build/1.0.1122/job/4ki3h7p55hew82pn#L1157
I'll remove it later.

@cdeil

This comment has been minimized.

Member

cdeil commented Jul 21, 2016

Please auto-format the code in gammapy/spectrum/models.py (PEP8 stuff)

Concerning the API, can we please change to energy_band instead of emin and emax as separate parameters? Im my experience this is a little more convenient for users, to have fewer objects. It's also used e.g. here and here

@cdeil cdeil added the feature label Jul 21, 2016

@cdeil cdeil added this to the 0.5 milestone Jul 21, 2016

@cdeil cdeil self-assigned this Jul 21, 2016

@cdeil

This comment has been minimized.

Member

cdeil commented Jul 21, 2016

For Sherpa models, we should just do what @zblz started here (https://github.com/zblz/naima/blob/master/naima/sherpa_models.py#L74) and define our own Sherpa models. Yes, a few of the ones we want are built-in, but they don't have the same parameter names and sometimes not the same parametrisation.

So just putting our own seems easiest to me.

@adonath

This comment has been minimized.

Member

adonath commented Jul 22, 2016

@cdeil Thanks! I've addressed your comments.

Concerning the sherpa models: I just had a quick look at the spectral models in sherpa and couldn't find an exponential cut off (except as xspec model, but with different parametrization and only available when xspec is installed...). For now I'll leave the to_sherpa() method unimplemented for the ExponentialCutoffPowerLaw model.

@cdeil

This comment has been minimized.

Member

cdeil commented Jul 22, 2016

For now I'll leave the to_sherpa() method unimplemented for the ExponentialCutoffPowerLaw model.

👍

Merge?

@adonath adonath merged commit 12abcd5 into gammapy:master Jul 22, 2016

2 checks passed

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

This comment has been minimized.

Contributor

joleroi commented Jul 25, 2016

@adonath I think the XSPEC exponential cutoff PL is all that's there. So I agree that it's best to write down our own version/parametrization of the ECPL as @cdeil suggested.

@adonath adonath deleted the adonath:spectral_model_improvements branch Nov 20, 2018

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