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

Activate EDispMap in MapEvaluator #2587

Merged
merged 10 commits into from Nov 21, 2019
Merged

Conversation

@adonath
Copy link
Member

adonath commented Nov 21, 2019

Description

This work was done pair-coding with @AtreyeeS. This pull request changes to allow using an EDispMap in the MapEvaluator. To simplify the adaption we allow for both an EnergyDispersion and EDispMap object for now. The following tests we already adapted to use the EDispMap:

  • gammapy/cube/tests/test_fit.py
  • gammapy/spectrum/tests/test_flux_points_estimator.py
  • gammapy/time/tests/test_lightcurve.py

The change in the flux point estimator and lightcurve tests are due to to fact of using simulate_dataset() to generate the test data. The IRFs were taken at an offset of 1 deg
independent of the position of the model component. After this PR the "correct" edisp (defined
at the position of the model component) is taken.

This PR also adds the following helper functions:

  • PSFMap.from_energy_dependent_table_psf()
  • EDsipMap.from_diagonal_response()

In follow-up PRs we will adapt to use PSFMap as well and probably disallow to an EnergyDispersion object for the MapDataset.


@pytest.mark.parametrize(
"position", [
SkyCoord("0 deg", "0 deg"),

This comment has been minimized.

Copy link
@cdeil

cdeil Nov 21, 2019

Member

You can just put ["0d 0d", "180d 0d", ...] here, and then put SkyCoord(position) once in the code below, might on one line then. (same in the test above).
And I think you have to add @requires_data.

@cdeil cdeil added this to In progress in gammapy.cube via automation Nov 21, 2019
@cdeil cdeil added the feature label Nov 21, 2019
@cdeil cdeil added this to the 0.15 milestone Nov 21, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Nov 21, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@f18b29f). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2587   +/-   ##
=========================================
  Coverage          ?   91.53%           
=========================================
  Files             ?      140           
  Lines             ?    16062           
  Branches          ?        0           
=========================================
  Hits              ?    14703           
  Misses            ?     1359           
  Partials          ?        0
Impacted Files Coverage Δ
gammapy/analysis/core.py 94.33% <ø> (ø)
gammapy/cube/psf_map.py 95.76% <100%> (ø)
gammapy/cube/edisp_map.py 95.45% <100%> (ø)
gammapy/cube/fit.py 89.7% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f18b29f...6250aad. Read the comment docs.

@adonath adonath merged commit 8fc61f6 into gammapy:master Nov 21, 2019
7 of 10 checks passed
7 of 10 checks passed
greeting
Details
Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
gammapy.gammapy in progress
Details
Scrutinizer Analysis: 5 updated code elements – Tests: passed
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
gammapy.cube automation moved this from In progress to Done Nov 21, 2019
This was referenced Nov 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
gammapy.cube
  
Done
2 participants
You can’t perform that action at this time.