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 combined 3D model and simple npred function #1040

Merged
merged 12 commits into from May 21, 2017

Conversation

Projects
None yet
1 participant
@cdeil
Member

cdeil commented May 20, 2017

This pull request is a continuation of #1029 where @robertazanin and I started to implement an example for 3D model evaluation and npred computation (and eventually simulating / fitting 3D).

What I have so far is moved the following things from the example script into Gammapy:

  • Add gammapy.cube.CombinedModel3D and some tests
  • Add gammapy.cube.compute_npred_cube_simple

What I still plan to do in this pull request:

  • Clean up the docstrings of CombinedModel3D
  • Complete the unit tests for CombinedModel3D
  • Clean up implementation and docstrings of the npred functions
  • Add tests for the npred computation functions (and debug the differences)

The next steps are then this, which I'll leave to future PRs:

  • Implement apply PSF for 3D and add to the example (probably refactor existing code, introduce a 3D PSF kernel class)
  • Implement apply EDISP and add to the example (probably refactor existing code in _sherpa.py
  • Introduce a base class for the spatial models using the Gammapy parameter class, so that in the combined model we can have the combined list of parameters and eventually can fit 3D

If anyone is interested in helping to implement something in this direction, please let me know.

@adonath - If you have time to review, please do. (I'll probably merge soon and start with follow-up PRs, but comments are welcome any time and then taken into account there).

@cdeil cdeil added the feature label May 20, 2017

@cdeil cdeil added this to the 0.7 milestone May 20, 2017

@cdeil cdeil self-assigned this May 20, 2017

@cdeil cdeil requested a review from adonath May 20, 2017

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil May 21, 2017

Member

I didn't get as far as I wanted today, but I'm still merging this now.

There was a bug in SkyCube.empty_like where the requested unit wasn't set, because internally np.ones_like was called, which (at least on my recent Numpy / Astropy) copies the units from the reference image or cube over.

Member

cdeil commented May 21, 2017

I didn't get as far as I wanted today, but I'm still merging this now.

There was a bug in SkyCube.empty_like where the requested unit wasn't set, because internally np.ones_like was called, which (at least on my recent Numpy / Astropy) copies the units from the reference image or cube over.

@cdeil cdeil merged commit 9fd86eb into gammapy:master May 21, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment