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 Fermi catalog spectrum evaluation and plotting #507

Merged
merged 7 commits into from Apr 11, 2016

Conversation

@joleroi
Copy link
Contributor

@joleroi joleroi commented Apr 11, 2016

This PR introduces

  • Integral and differential flux points Table subclasses
  • Conversion method from int to diff (wrapper around existing functionality)
  • read_3FGL methods for both flux points classes as well as SpectrumFitResult
  • evaluate and evaluate_butterfly for SpectrumFitResult

I replaces #504

Up next

  • The same for 2FHL (this is a little bit more involved)
@joleroi
Copy link
Contributor Author

@joleroi joleroi commented Apr 11, 2016

I will merge this if tests pass - if you want to comment, hurry up 😉

@cdeil cdeil added this to the 0.4 milestone Apr 11, 2016
out = reproject_interp(input_data=self.to_image_hdu(),
output_projection=refheader, *args, **kwargs)
map = SkyMap(name=self.name, data=out[0], wcs=self.wcs, unit=self.unit,
meta=self.meta)

This comment has been minimized.

@cdeil

cdeil Apr 11, 2016
Member

This is an old change, no?
Why does it appear again in this PR?

This comment has been minimized.

@joleroi

joleroi Apr 11, 2016
Author Contributor

Because I cherry picked the commit, to be able to run tests

This comment has been minimized.

@cdeil

cdeil Apr 11, 2016
Member

Hmmm, 🍒

This comment has been minimized.

@joleroi

joleroi Apr 11, 2016
Author Contributor

😋

parameters['norm'] = Quantity(d['Flux_Density'], 'cm-2 s-1 MeV-1')
parameter_errors['norm'] = Quantity(d['Unc_Flux_Density'],
'cm-2 s-1 MeV-1')
else:

This comment has been minimized.

@cdeil

cdeil Apr 11, 2016
Member

If you have time, run unique on that column and add a TODO comment here which are the missing spectral models.
This kind of stuff is a good task for people to get started, but it has to be clear what exactly needs to be done.

This comment has been minimized.

@joleroi

joleroi Apr 11, 2016
Author Contributor

I don't understand, sorry. What does this have to do with
http://docs.astropy.org/en/stable/api/astropy.table.unique.html
I am not using a table here, am I?

This comment has been minimized.

@cdeil

cdeil Apr 11, 2016
Member

I meant call unique on the Fermi catalog flux model column to get a complete list of all models there are, so that it's clear what further work is needed.
Not call unique here in the code.

flux_unit='cm-2 s-1 TeV-1', e_power=0, **kwargs):
"""Plot fit function
def evaluate(self, x):
"""Wrapper around astropy.modelling.models.evaluate

This comment has been minimized.

@cdeil

cdeil Apr 11, 2016
Member

I'd suggest this:
"""Wrapper around the evaluate method on the Astropy model classes.
"""

self.parameters.reference,
self.parameters.index)
elif self.spectral_model == 'LogParabola':
#LogParabola evaluation does not work with arrays

This comment has been minimized.

@cdeil

cdeil Apr 11, 2016
Member

What!?

Can you post a minimal code snippet here that shows that it doesn't work?

Leave a TODO here or directly file an issue or PR with Astropy?

This comment has been minimized.

@joleroi

joleroi Apr 11, 2016
Author Contributor

I added a comment
see astropy/astropy#4764

@cdeil
Copy link
Member

@cdeil cdeil commented Apr 11, 2016

Maybe add a test that exercises the plot_butterfly method?
(no asserts for now, just to make sure it runs fine for all builds)

@cdeil
Copy link
Member

@cdeil cdeil commented Apr 11, 2016

Ha ha ... I did manage to comment here before it went in. 😼

@joleroi
Copy link
Contributor Author

@joleroi joleroi commented Apr 11, 2016

Yes the test situation with the results module is horrible. I will add some test in a follow-up PR (already in progress)

@joleroi joleroi force-pushed the joleroi:flux_pointsv2 branch from 5d6c2b8 to a26c36b Apr 11, 2016
@joleroi joleroi merged commit dfeab4d into gammapy:master Apr 11, 2016
2 checks passed
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:flux_pointsv2 branch Apr 11, 2016
@cdeil cdeil changed the title Add convenience methods to read 3FGL Add Fermi catalog spectrum evaluation and plotting Apr 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants