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

Add MapDatasetEventSampler.sample_background() method #2616

Merged
merged 6 commits into from Nov 28, 2019

Conversation

@fabiopintore
Copy link
Contributor

fabiopintore commented Nov 27, 2019

I am introducing the new class MapDatasetEventSampler into ~gammapy.cube.simulate and its method sample_background.
The class is needed for the high level interface for the event sampler (PIG 9), while the method is developed to sample the events of a given background map.

@adonath adonath self-assigned this Nov 27, 2019
@adonath adonath added the feature label Nov 27, 2019
@adonath adonath added this to the 0.15 milestone Nov 27, 2019
Copy link
Member

adonath left a comment

Thanks @fabiopintore, I've left a fe minor inline comments...pleas address, before we can merge.

from gammapy.maps import WcsNDMap
from gammapy.modeling.models import BackgroundModel
from gammapy.modeling.models import (
ConstantTemporalModel,
LightCurveTemplateTemporalModel,

This comment has been minimized.

Copy link
@adonath

adonath Nov 27, 2019

Member

I think the only used import is ConstantTemporalModel please remove the others...

This comment has been minimized.

Copy link
@fabiopintore

fabiopintore Nov 28, 2019

Author Contributor

Done


def sample_background(self, dataset):
"""Sample background
Parameters

This comment has been minimized.

Copy link
@adonath

adonath Nov 27, 2019

Member

Missing empty line before Parameters

This comment has been minimized.

Copy link
@fabiopintore

fabiopintore Nov 28, 2019

Author Contributor

Done

Parameters
----------
dataset : `MapDataset`
Map dataset.

This comment has been minimized.

Copy link
@adonath

adonath Nov 27, 2019

Member

Indent this line

This comment has been minimized.

Copy link
@fabiopintore

fabiopintore Nov 28, 2019

Author Contributor

Done

# sample position
coords = background.sample_coord(n_events, self.random_state)
table["ENERGY"] = coords["energy"]
table["RA"] = coords["lon"]

This comment has been minimized.

Copy link
@adonath

adonath Nov 27, 2019

Member

We have to make sure, that the coordinates are really given in ices. For this you can use coords.skycoord.icrs.ra.deg and coords.skycoord.icrs.dec.deg

This comment has been minimized.

Copy link
@fabiopintore

fabiopintore Nov 28, 2019

Author Contributor

Done

1, 100, nbin=30, unit="TeV", name="energy", interp="log"
)

exposure = Map.create(

This comment has been minimized.

Copy link
@adonath

adonath Nov 27, 2019

Member

It seems exposure is not used, just remove...

This comment has been minimized.

Copy link
@fabiopintore

fabiopintore Nov 28, 2019

Author Contributor

Done


return dataset


This comment has been minimized.

Copy link
@adonath

adonath Nov 27, 2019

Member

I think this test requires a @requires_data() decorator. In Gammapy we have to mark tests, which rely on test data in $GAMMAPY_DATA (the cat IRFs in this case...). You can import the @requires_data() decorator from from gammapy.utils.tests import requires_data

@codecov

This comment has been minimized.

Copy link

codecov bot commented Nov 28, 2019

Codecov Report

Merging #2616 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2616      +/-   ##
=========================================
+ Coverage   91.49%   91.5%   +0.01%     
=========================================
  Files         141     141              
  Lines       15974   15939      -35     
=========================================
- Hits        14615   14585      -30     
+ Misses       1359    1354       -5
Impacted Files Coverage Δ
gammapy/cube/simulate.py 97.72% <100%> (+2.07%) ⬆️
gammapy/modeling/models/spatial.py 97.4% <0%> (-1.74%) ⬇️
gammapy/catalog/gammacat.py 90.53% <0%> (-0.53%) ⬇️
gammapy/modeling/models/cube.py 94.67% <0%> (-0.25%) ⬇️
gammapy/catalog/fermi.py 95.39% <0%> (ø) ⬆️
gammapy/catalog/hess.py 97.18% <0%> (ø) ⬆️
gammapy/catalog/hawc.py 98.76% <0%> (ø) ⬆️
gammapy/modeling/serialize.py 98.86% <0%> (+0.01%) ⬆️
gammapy/spectrum/flux_point.py 91.28% <0%> (+0.08%) ⬆️
gammapy/spectrum/dataset.py 95.56% <0%> (+0.2%) ⬆️
... and 2 more

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 360f523...04fecb2. Read the comment docs.

fabiopintore and others added 2 commits Nov 28, 2019
@adonath adonath changed the title Add sample_background() to MapDatasetEventSampler Add MapDatasetEventSampler with .sample_background() method Nov 28, 2019
@adonath adonath merged commit 90e00d0 into gammapy:master Nov 28, 2019
12 checks passed
12 checks passed
greeting
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Scrutinizer Analysis: 4 updated code elements – Tests: passed
Details
codecov/patch 100% of diff hit (target 91.49%)
Details
codecov/project 91.5% (+0.01%) compared to 360f523
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
gammapy.gammapy #20191128.18 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
@adonath adonath changed the title Add MapDatasetEventSampler with .sample_background() method Add MapDatasetEventSampler.sample_background() method Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.