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

Time handling in Gammapy #284

Open
cdeil opened this issue Jun 12, 2015 · 3 comments

Comments

@cdeil
Copy link
Member

@cdeil cdeil commented Jun 12, 2015

In Gammapy we currently use the utc time scale. I think in CTA it is under discussion if the utc or tt time scale should be used (e.g. for event times, but really all times).

See http://astropy.readthedocs.org/en/latest/time/index.html#time-scale and references there.

@kosack Do you know what the status of the UTC versus TT time scale discussion is in CTA? Even if it's not written up in a spec yet, what should we use in Gammapy for now? Or will the time scale always be put in the FITS header and CTA software (science tools specifically) is supposed to handle both UTC and TT?

TODO:

  • Look at times in Gammalib and make sure that what we do is consistent (at least when it comes to files that are being exchanged).
  • Decide the time scale
  • Update code and docs
  • Implement MET converter functions "the right way".

I think MET should be implemented by defining a time scale (see TimeCxcSec as an example)) ?

@cdeil cdeil self-assigned this Jun 12, 2015
@cdeil cdeil added this to the whishlist milestone Jun 12, 2015
@cdeil cdeil changed the title Use utc or tt time scale? Time handling in Gammapy Jun 12, 2015
@kosack

This comment has been minimized.

Copy link

@kosack kosack commented Jun 18, 2015

I think we should use a time scale that doesn't need leap-second corrections (so we should not use UTC). That makes making lightcurves, etc, much easier. Otherwise, we rely on a leap-second database to be updated and maintained. I think that's why TT is used normally, and you only go to UTC when needed (e.g. for displaying a human-readable time, etc). I may be wrong though.

@cdeil cdeil modified the milestones: wishlist, 0.13 May 29, 2019
@cdeil cdeil modified the milestones: 0.13, 0.14 Jul 18, 2019
@cdeil cdeil modified the milestones: 0.14, 0.15 Sep 5, 2019
@cdeil cdeil assigned registerrier and unassigned cdeil Nov 22, 2019
@cdeil

This comment has been minimized.

Copy link
Member Author

@cdeil cdeil commented Nov 22, 2019

@registerrier - Could you please clean up the docs and code how to work with times in Gammapy?

Code-wise, my main suggestion is to use Table objects with START and STOP Time columns, instead of Python lists for time intervals in the LC API. This will let you delete this code:

def _check_and_sort_time_intervals(self, time_intervals):
"""Sort the time_intervals by increasing time if not already ordered correctly.
Parameters
----------
time_intervals : list of `astropy.time.Time`
Start and stop time for each interval to compute the LC
"""
time_start = Time([interval[0] for interval in time_intervals])
time_stop = Time([interval[1] for interval in time_intervals])
sorted_indices = time_start.argsort()
time_start_sorted = time_start[sorted_indices]
time_stop_sorted = time_stop[sorted_indices]
diff_time_stop = np.diff(time_stop_sorted)
diff_time_interval_edges = time_start_sorted[1:] - time_stop_sorted[:-1]
if np.any(diff_time_stop < 0) or np.any(diff_time_interval_edges < 0):
raise ValueError("LightCurveEstimator requires non-overlapping time bins.")
else:
self.time_intervals = [
Time([tstart, tstop])
for tstart, tstop in zip(time_start_sorted, time_stop_sorted)
]

Astropy has supported Time columns in Table for ~ 5 years:
http://docs.astropy.org/en/stable/table/mixin_columns.html#mixin-columns

There's probably subtleties when reading & writing, and whether to use Table or QTable. We can discuss those and find a solution, but generally the direction should be to use table and time objects.

Concerning documentation, maybe one idea would be to delete this:
https://docs.gammapy.org/0.14/utils/index.html#time-handling-in-gammapy
and instead put a (short) section explaining how to work with times here?
https://docs.gammapy.org/0.14/time/index.html

This time ref we should probably delete:

# TODO: implement and document this properly.
# see https://github.com/gammapy/gammapy/issues/284
TIME_REF_FERMI = Time("2001-01-01T00:00:00")

It can be in our tests, but it should be in our utils.

If we want to keep the time ref points for Fermi, HESS, CTA, ... we should probaby move them here and maintain it together with the ref locations (not set for Fermi obviously):
https://github.com/gammapy/gammapy/blob/master/gammapy/data/observers.py

@registerrier - Thanks for cleaning that up!
Let me know if issues arise ...

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