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 function to compute background cube #1183

Merged
merged 5 commits into from Oct 23, 2017

Conversation

Projects
None yet
2 participants
@cdeil
Member

cdeil commented Oct 23, 2017

This PR adds a helper function make_background_cube, which fills a background cube model into a SkyCube. We also add a bin_size property to SkyCube, which gives the sky cube cell bin size, i.e. energy width times solid angle. This is continuing the work started in #1162 where the gammapy.irf.Background3D class was added.

We plan to change to gammapy.maps for these functions and cube analysis soon, but while that is still under development we continue with SkyCube for now.

The goal for the next days is to get a CTA GPS background model image using these background models.

Pair coding with @robertazanin .

cc @adonath @bkhelifi

@cdeil cdeil added the feature label Oct 23, 2017

@cdeil cdeil added this to the 0.7 milestone Oct 23, 2017

@cdeil cdeil self-assigned this Oct 23, 2017

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 23, 2017

Member

We've started an example how to compute a background cube survey image in gammasky/cta-dc1-gps-analysis@6259dd6 . @robertazanin will continue with this, scripting the cutout / paste and energy integration. Here we had to change the background loading via DataStoreObservation from the old FOVCube to the new Background3D. Doesn't look like there were any callers / users, so I think it's OK. (if there are complaints, we'll just implement what's needed on the new class).

@adonath - If you have time to help with this, adding a SkyCube.paste method would be helpful. (we want / need 3D survey images and analysis as well short-term, not just images).

I'll leave this open for comments now, and then merge tonight if there are none. @adonath - Maybe you have time to review this?

Member

cdeil commented Oct 23, 2017

We've started an example how to compute a background cube survey image in gammasky/cta-dc1-gps-analysis@6259dd6 . @robertazanin will continue with this, scripting the cutout / paste and energy integration. Here we had to change the background loading via DataStoreObservation from the old FOVCube to the new Background3D. Doesn't look like there were any callers / users, so I think it's OK. (if there are complaints, we'll just implement what's needed on the new class).

@adonath - If you have time to help with this, adding a SkyCube.paste method would be helpful. (we want / need 3D survey images and analysis as well short-term, not just images).

I'll leave this open for comments now, and then merge tonight if there are none. @adonath - Maybe you have time to review this?

@adonath

I've left two minor inline comments. This function has been missing for a long time so it's good to have it finally implemented.

Show outdated Hide outdated gammapy/cube/exposure.py Outdated
data = data.to('')
return SkyCube(

This comment has been minimized.

@adonath

adonath Oct 23, 2017

Member

Totally minor: one could use SkyCube.empty_like(ref_cube) to save a few lines...

@adonath

adonath Oct 23, 2017

Member

Totally minor: one could use SkyCube.empty_like(ref_cube) to save a few lines...

This comment has been minimized.

@cdeil

cdeil Oct 23, 2017

Member

I don't think it really helps. I would have to call empty_like, and then set the name on a second line and copy the data over on a third, no? I prefer this coding style, but what I have now would easily fit on two lines.

@cdeil

cdeil Oct 23, 2017

Member

I don't think it really helps. I would have to call empty_like, and then set the name on a second line and copy the data over on a third, no? I prefer this coding style, but what I have now would easily fit on two lines.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 23, 2017

Member

@adonath - Thanks for the review. I've done some cleanup in 4aa9ba4 . Merging this now.

Note that in open-gamma-ray-astro/gamma-astro-data-formats#97 I confirmed that background rate is really per obs time, not per live time. I'll make a small edit to the spec now to make this more clear.

Member

cdeil commented Oct 23, 2017

@adonath - Thanks for the review. I've done some cleanup in 4aa9ba4 . Merging this now.

Note that in open-gamma-ray-astro/gamma-astro-data-formats#97 I confirmed that background rate is really per obs time, not per live time. I'll make a small edit to the spec now to make this more clear.

@cdeil cdeil merged commit 8c4ef24 into gammapy:master Oct 23, 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