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 3D model simulation example #1029

Merged
merged 3 commits into from May 20, 2017

Conversation

Projects
None yet
3 participants
@robertazanin
Contributor

robertazanin commented May 17, 2017

This pull request adds a 3D dataset simulation example using CTA IRFs.

robertazanin robertazanin

@cdeil cdeil self-assigned this May 17, 2017

@cdeil cdeil added the feature label May 17, 2017

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

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil May 17, 2017

Member

@robertazanin - I continued a bit with this and pushed two commits here.

I also remembered that Fabio and Lea started the same exercise during the Paris workshop:
https://github.com/facero/gammapy-extra/blob/fd3321833dbd77b0ff63e002590c0450c912a37e/notebooks/cta_obs_sim.ipynb
but didn't finish it, the pull request is still open:
gammapy/gammapy-extra#75
And it looks like they used Sherpa extensively, at least for the models, and I think they are duplicating quite a bit of code that's already in gammapy.cube

There's the fundamental question here if we should use Sherpa or just continue implementing this ourselves in Gammapy just using Numpy. My feeling is that the Sherpa 3D stuff is very complex, and continuing with the simple 3D model and npred implementation just using Numpy and Gammapy we have started here could be useful (and once available we can discuss which version we want to keep and maintain in Gammapy, either the Sherpa or homegrown or both). So I'd say we continue with this example a bit, and try to split out clearly useful parts into the Gammapy library.

The next steps are to apply the PSF convolution and the EDISP.
@adonath - do we have non-Sherpa code to do this already in Gammapy using just Numpy arrays?
Would it be a quick addition for you to add to this working example script or should we do it from scratch tomorrow?

Member

cdeil commented May 17, 2017

@robertazanin - I continued a bit with this and pushed two commits here.

I also remembered that Fabio and Lea started the same exercise during the Paris workshop:
https://github.com/facero/gammapy-extra/blob/fd3321833dbd77b0ff63e002590c0450c912a37e/notebooks/cta_obs_sim.ipynb
but didn't finish it, the pull request is still open:
gammapy/gammapy-extra#75
And it looks like they used Sherpa extensively, at least for the models, and I think they are duplicating quite a bit of code that's already in gammapy.cube

There's the fundamental question here if we should use Sherpa or just continue implementing this ourselves in Gammapy just using Numpy. My feeling is that the Sherpa 3D stuff is very complex, and continuing with the simple 3D model and npred implementation just using Numpy and Gammapy we have started here could be useful (and once available we can discuss which version we want to keep and maintain in Gammapy, either the Sherpa or homegrown or both). So I'd say we continue with this example a bit, and try to split out clearly useful parts into the Gammapy library.

The next steps are to apply the PSF convolution and the EDISP.
@adonath - do we have non-Sherpa code to do this already in Gammapy using just Numpy arrays?
Would it be a quick addition for you to add to this working example script or should we do it from scratch tomorrow?

@adonath

This comment has been minimized.

Show comment
Hide comment
@adonath

adonath May 18, 2017

Member

@cdeil +1 to reimplement this with numpy and split out useful parts into Gammapy. I think that's what we've also been doing for images and spectra.

PSF convolution works via (pseudo code...):

psf = EnergyDependentTablePSF()
cube = SkyCube.empty()
kernels = psf.kernels(cube)
cube_convolved = cube.convolve(kernels)

Applying energy dispersion doesn't exist...

Member

adonath commented May 18, 2017

@cdeil +1 to reimplement this with numpy and split out useful parts into Gammapy. I think that's what we've also been doing for images and spectra.

PSF convolution works via (pseudo code...):

psf = EnergyDependentTablePSF()
cube = SkyCube.empty()
kernels = psf.kernels(cube)
cube_convolved = cube.convolve(kernels)

Applying energy dispersion doesn't exist...

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil May 20, 2017

Member

I'm merging this now and will continue with this example, and with refactoring the parts developed here into the Gammapy package in follow-up pull requests.

@robertazanin - Thanks!

Member

cdeil commented May 20, 2017

I'm merging this now and will continue with this example, and with refactoring the parts developed here into the Gammapy package in follow-up pull requests.

@robertazanin - Thanks!

@cdeil cdeil merged commit 01e1447 into gammapy:master May 20, 2017

2 checks passed

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

@cdeil cdeil referenced this pull request May 20, 2017

Merged

Add combined 3D model and simple npred function #1040

6 of 6 tasks complete

@cdeil cdeil changed the title from Add 3D simulator example to Add 3D model simulation example May 21, 2017

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