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

Speed up Observations.select_time #2580

Merged
merged 4 commits into from Nov 21, 2019
Merged

Conversation

@registerrier
Copy link
Contributor

registerrier commented Nov 21, 2019

Description

This pull request modifies the implementation of Observations.select_time to make it run faster. It simply creates Time objects containing start and stop times for all observations in the list and creates a mask to find which ones should be filtered.

On a simple test on PKS 2155 data from hess-dl3-dr1, it was found to speed up observations time selection by a factor of 10.
For selections over very large Observations, this can be a significant improvement for observation selection.

Dear reviewer

the PR is ready for review.

@registerrier registerrier added this to To do in gammapy.time via automation Nov 21, 2019
@registerrier registerrier 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

Merging #2580 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2580   +/-   ##
======================================
  Coverage    91.5%   91.5%           
======================================
  Files         140     140           
  Lines       15971   15971           
======================================
  Hits        14615   14615           
  Misses       1356    1356
Impacted Files Coverage Δ
gammapy/data/observations.py 77.73% <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 dae94c5...b1d48c7. Read the comment docs.

@cdeil cdeil self-assigned this Nov 21, 2019
@cdeil
cdeil approved these changes Nov 21, 2019
Copy link
Member

cdeil left a comment

I didn't know & works for bool, but it looks like it does. Merging.

@cdeil cdeil merged commit 4de2416 into gammapy:master Nov 21, 2019
12 checks passed
12 checks passed
greeting
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Scrutinizer Analysis: No new issues – Tests: passed
Details
codecov/patch 100% of diff hit (target 91.5%)
Details
codecov/project 91.5% (+0%) compared to dae94c5
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
gammapy.gammapy Build #20191121.6 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
gammapy.time automation moved this from To do to Done Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
gammapy.time
  
Done
2 participants
You can’t perform that action at this time.