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

Adapt MapMakerObs to also compute an EDispMap and PSFMap #2358

Merged
merged 9 commits into from Sep 13, 2019

Conversation

@AtreyeeS
Copy link
Member

commented Sep 9, 2019

Hello @adonath @registerrier @luca-giunti ! This PR addresses one of the steps towards dataset stacking.

I had to modify MapMaker a bit to ensure tests do not fail. This is mostly a patchwork, as MapMaker will go away soon. I am not sure how the sum_over_axes() should work for the psf, and edisp maps. Since stackings are being implemented separately, I did not do the stacking of the maps in the MapMaker either.

The CTA DC1 psf files are gammapy.irf.psf_gauss.EnergyDependentMultiGaussPSF instead of gammapy.irf.psf_3d.PSF3D. I do not know how to handle this, so for now, I have just put a check.

@adonath adonath self-assigned this Sep 10, 2019
@adonath adonath added the feature label Sep 10, 2019
@adonath adonath added this to the 0.14 milestone Sep 10, 2019
gammapy/cube/make.py Outdated Show resolved Hide resolved
Copy link
Member

left a comment

Thanks @AtreyeeS! I've left some comments, please address those before we can merge.

gammapy/cube/make.py Show resolved Hide resolved
gammapy/cube/make.py Outdated Show resolved Hide resolved
gammapy/cube/make.py Show resolved Hide resolved
gammapy/cube/make.py Outdated Show resolved Hide resolved
gammapy/cube/make.py Outdated Show resolved Hide resolved
gammapy/cube/make.py Outdated Show resolved Hide resolved
gammapy/cube/make.py Outdated Show resolved Hide resolved
gammapy/cube/make.py Outdated Show resolved Hide resolved
AtreyeeS added 2 commits Sep 10, 2019
@AtreyeeS

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2019

Thanks @adonath ! I have addressed the changes you suggested

Copy link
Contributor

left a comment

Thanks @AtreyeeS . Looks good.

Some minor comments/remarks inline.

gammapy/cube/make.py Show resolved Hide resolved
gammapy/cube/make.py Outdated Show resolved Hide resolved
gammapy/cube/make.py Outdated Show resolved Hide resolved
gammapy/cube/make.py Outdated Show resolved Hide resolved
gammapy/cube/psf_map.py Outdated Show resolved Hide resolved
@registerrier registerrier merged commit 435798b into gammapy:master Sep 13, 2019
8 of 9 checks passed
8 of 9 checks passed
Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
Scrutinizer Analysis: 4 updated code elements – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
gammapy.gammapy Build #20190913.10 succeeded
Details
gammapy.gammapy (DevDocs) DevDocs succeeded
Details
gammapy.gammapy (Lint) Lint succeeded
Details
gammapy.gammapy (Test Python36) Test Python36 succeeded
Details
gammapy.gammapy (Test Windows36) Test Windows36 succeeded
Details
gammapy.gammapy (Test Windows37) Test Windows37 succeeded
Details
@registerrier

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2019

Issues are solved, Travis passes. Merging.
Thanks @AtreyeeS !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.