Ticket #800 (closed defect: fixed)

Opened 10 years ago

Last modified 10 years ago

DateTime constructor accepts ridiculous MJDs

Reported by: fpierfed Owned by: ktl
Priority: normal Milestone:
Component: daf_base Keywords:
Cc: Blocked By:
Blocking: Project: LSST
Version Number:
How to repeat:
lsst6> cat test.py
#!/usr/bin/env python
import lsst.afw.detection as det
from lsst.daf.base import DateTime
t = 1209102482808000000  # from the database
d = det.DiaSource()
d.setTaiMidPoint(t)
print(DateTime(d.getTaiMidPoint()).mjd(DateTime.TAI))
print(DateTime(t).mjd(DateTime.TAI))
lsst6> python test.py
-66164.9911673
54581.2416992

Description (last modified by ktl) (diff)

If a Python long representing nanoseconds since the epoch is passed to a DateTime constructor expecting an MJD as a double, no exception occurs, and the resulting DateTime can return a very different value from its mjd() accessor due to overflow of the internal 64-bit value.

Change History

comment:1 Changed 10 years ago by rhl

I don't understand. !taiMidPoint is a double in the code, and I'd have thought that this was correct. Are you saying that it should be a long everywhere? If so, what's the reason?

You don't provide enough information for me to know when python is returning a negative number, and I'm rather surprised; I wouldn't expect that if the C++ consistently makes it a double (a conversion to long could do it, but I don't see one)

comment:2 Changed 10 years ago by fpierfed

  • Owner changed from rhl to ktl
  • Component changed from afw to daf_base

You are right: the problem is a bad interaction with daf.base.Datetime and probably not an issue with afw:

lsst6> cat test.py
#!/usr/bin/env python
import lsst.afw.detection as det
from lsst.daf.base import DateTime

t = 1209102482808000000  # from the database
d = det.DiaSource()
d.setTaiMidPoint(t)

print(DateTime(d.getTaiMidPoint()).mjd(DateTime.TAI))
print(DateTime(t).mjd(DateTime.TAI))

lsst6> python test.py
-66164.9911673
54581.2416992

comment:3 follow-up: ↓ 4 Changed 10 years ago by ktl

DIASource.taiMidPoint is a double in the database schema. The formatter persists it as a double and binds it as a double on input. As far as I can tell, DC3a does not set this field at all. Where in the code is it getting set as nanoseconds? (Of course your test script fails if it is setting a double field intended for an MJD with an int64 number of nanoseconds.)

comment:4 in reply to: ↑ 3 Changed 10 years ago by fpierfed

I am not sure I agree that it is normal either for that test to fail or for it to produce a negative number. This regardless of the value of t. In particular, if t is a float, DateTime(1209102482808000000.).mjd(DateTime.TAI) = -66164.9911673 which does not make sense.

comment:5 Changed 10 years ago by ktl

The point is that setTaiMidPoint() takes an MJD, not a DateTime or a DateTime.nsecs().

DateTime(1209102482808000000.0) doesn't make sense, either, as the double constructor also takes an MJD.

comment:6 Changed 10 years ago by fpierfed

:-)

I guess we are talking philosophy here. My point is that there is no reason that 1209102482808000000.0 is not a valid MJD. It is a float and corresponds to a date very far in the future. If I init a DateTime? with a MJD and then ask for the corresponding MJD back, I should get the same float I used to init the object, no?

If we decide that 1209102482808000000.0 is not a valid MJD, then wouldn't it be nice for DateTime? to inform the user of that decision?

comment:7 Changed 10 years ago by ktl

  • Status changed from new to assigned

OK, I can throw an exception for any date more than 4G seconds after the epoch (2106 or so).

comment:8 Changed 10 years ago by ktl

Correction: since the internal representation is a signed long long in nanoseconds, the valid date range is approximately 8G seconds on either side of the epoch. This is approximately the years 1717 to 2223.

comment:9 Changed 10 years ago by ktl

  • Status changed from assigned to closed
  • How to repeat modified (diff)
  • Resolution set to fixed
  • Description modified (diff)
  • Summary changed from BaseSourceAttributes/DiaSource.setTaiMidPoint() uses a double instead of a long to DateTime constructor accepts ridiculous MJDs

Fixed directly on trunk, with several test cases, in [9765]. This checks the following constructors:

  • DateTime(double mjd, Timescale): MJD must be within 106751.99 days of the epoch.
  • DateTime(int, int, int, int, int, int, Timescale): date must be convertible to time_t by Unix mktime() (within 2G seconds of the epoch).
  • DateTime(std::string): date must be convertible to time_t by Unix mktime() (within 2G seconds of the epoch).

In addition, exceptions now contain the failing parameter value.

Note: See TracTickets for help on using tickets.