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

Ensure events are sorted in time order #425

Closed
JonathanDHarris opened this issue Jan 25, 2016 · 3 comments
Closed

Ensure events are sorted in time order #425

JonathanDHarris opened this issue Jan 25, 2016 · 3 comments
Assignees
Milestone

Comments

@JonathanDHarris
Copy link
Contributor

Some code will require that event lists are in time order.

We should update the docs to reflect this
http://gamma-astro-data-formats.readthedocs.org/en/latest/events/index.html#required-column-names

and add to the check_times method
https://github.com/gammapy/gammapy/blob/master/gammapy/data/event_list.py#L721

I'll probably pick this issue up soon if no one else wants it

@cdeil cdeil added this to the 0.5 milestone Feb 20, 2016
@cdeil cdeil modified the milestones: 0.6, 0.5 Jul 7, 2016
@cdeil cdeil modified the milestones: 0.6, 0.7 Jan 27, 2017
@cdeil cdeil modified the milestones: 0.7, 0.8 Sep 14, 2017
@cdeil cdeil modified the milestones: 0.8, 0.9 May 7, 2018
@cdeil
Copy link
Contributor

cdeil commented May 7, 2018

I've moved this discussion to the format spec:
open-gamma-ray-astro/gamma-astro-data-formats#107
and once that's resolved we can adapt Gammapy.

@cdeil cdeil modified the milestones: 0.9, 0.10 Oct 31, 2018
@cdeil cdeil modified the milestones: 0.10, 0.12 Jan 17, 2019
@cdeil cdeil modified the milestones: 0.12, 0.13 May 17, 2019
@cdeil cdeil modified the milestones: 0.13, 0.14 Jul 18, 2019
@cdeil cdeil modified the milestones: 0.14, 0.15 Sep 12, 2019
@cdeil
Copy link
Contributor

cdeil commented Oct 25, 2019

@adonath @registerrier - Let's try to resolve this old issue, please comment.

I think it won't be much work, but if we leave it unaddressed it probably means that some users get completely incorrect results without a warning. For sure we'll add a few lines of documentation, and concerning code changes it's either nothing, or a call to events.table.sort("TIME") or GTI table sort in one or a few places.

Do you know if there are any places in Gammapy where we rely on EVENTS or GTI being sorted by time?
Is gammapy.time.exptest the only one?
I think in lightcurve and other code, we use boolean masking and it doesn't matter how the tables are ordered?

It's not clear if we should support non-chronological EVENTS tables. The discussion in open-gamma-ray-astro/gamma-astro-data-formats#107 hasn't advanced, and probably it will take a long time for CTA to decide. So let's not wait for that. As far as I know, ctools does produce EVENTS that are non-sorted in ctobssim (at least it used to). I don't know about the Fermi tools.

If we don't rely on order: add docs saying so.

If we do rely on order in some places: we could sort at the start of those functions / methods, or raise an error. We could also sort on EVENTS (and GTI) read, although that isn't a good solution IMO.

Thoughts?

@cdeil cdeil self-assigned this Oct 25, 2019
@cdeil cdeil added this to To do in gammapy.data via automation Oct 25, 2019
@cdeil
Copy link
Contributor

cdeil commented Nov 22, 2019

I've added a note that we shouldn't rely on TIME being sorted in EVENTS here: 063a322

Otherwise I reviewed the code, there's no place where we rely on TIME sorting in EVENTS.

Closing this issue now.

Improving code and documentation concerning Time in Gammapy is covered by this issue:
#284 (comment)

@cdeil cdeil closed this as completed Nov 22, 2019
gammapy.data automation moved this from To do to Done Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
gammapy.data
  
Done
Development

No branches or pull requests

2 participants