Planning to change State.time to milliseconds

26 views
Skip to first unread message

Avi Flamholz

unread,
May 18, 2013, 1:14:38 PM5/18/13
to opentripp...@googlegroups.com, Andrew Byrd, David Turner
Hey all --

For those of you who don't know, OTP keeps time as a long and rounds up to the nearest second each time it traverses a PlainStreetEdge (see PlainStreetEdge.java). This rounding can lead to cases where a path's weight and its duration are off by up to a second per edge, which can make it hard to write tests that check the validity of our routing code.

I am planning to switch State.time and other related time variables to be in milliseconds. Does anyone know of a reason that I should not do this? 

Thanks,
Avi

nfn

unread,
May 18, 2013, 4:43:15 PM5/18/13
to opentripp...@googlegroups.com, Andrew Byrd, David Turner
I didn't understand what you mean. Can you explain better?
The time is used for what?

Stefan Steiniger

unread,
May 18, 2013, 6:39:03 PM5/18/13
to opentripp...@googlegroups.com
Isn't it provided as long by the Java system?
Though as "long" as other code does not need to be changed, e.g. API
requests (I think I parsed time within the shortest path tree for the
isochrones/walksheds in the analysis package).

Am 18.05.13 13:14, schrieb Avi Flamholz:
> --
> You received this message because you are subscribed to the Google
> Groups "OpenTripPlanner Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to opentripplanner...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

Andrew Byrd

unread,
May 18, 2013, 9:13:01 PM5/18/13
to nfn, opentripp...@googlegroups.com, David Turner
The first field in State object is a long integer called 'time', the
epoch time (in seconds) at which that state can be reached.

When we traverse a street edge, we do calculations using the edge
length, speed, etc. using floating point numbers and store the result
back into an integer in State, so some rounding must occur. See
PlainStreetEdge line 385.

-Andrew

Andrew Byrd

unread,
May 18, 2013, 9:28:46 PM5/18/13
to Avi Flamholz, opentripp...@googlegroups.com, David Turner
My first reaction was to wonder whether this actually affects results
much. But then, especially considering that we already use long integers
to store time:

The current time in seconds since the epoch is 1 368 921 705, requiring
10 significant decimal digits to maintain 1-second resolution.

2**63 is 9 223 372 036 854 775 808 (~19 decimal digits of precision), so
a signed long can hold time to roughly nanosecond resolution for the
next several hundred years.

2**31 is 2 147 483 648, so a signed int can hold time at 1-second
resolution for the next 24 years or so (not good).

Doubles give us 52 bits worth of significand, storing integers exactly
up to 9 007 199 254 740 992 (~16 significant decimal digits), and can
store time exactly at millisecond resolution for the next few hundred years.

So I don't see any real argument against storing it as milliseconds in
longs. Remember though that all data types are discrete and you will run
into roundoff error whatever the resolution. The discrepancies would be
less visible in itineraries (if they are even visible now), but could
still cause assertions based on analytic reasoning to fail if not
properly accounted for.

-Andrew

Andrew Byrd

unread,
May 19, 2013, 9:56:20 PM5/19/13
to Avi Flamholz, opentripp...@googlegroups.com, David Turner
I thought this seemed familiar. We used to store time as milliseconds
but switched to 1-sec resolution because of rounding problems (see
commit 488ccbeef and issue 455). We should be careful not to re-create
old problems.

Avi Flamholz

unread,
May 20, 2013, 11:42:26 AM5/20/13
to Andrew Byrd, opentripp...@googlegroups.com, David Turner
I agree. However, it seems like you have rounding problems whether you store in seconds or milliseconds. You have round either way to convert to seconds when you need to.

I am working on a change for this now. I will do my best to test it, but I hope you and David can also take a look and perhaps add some tests for the transit case?

A
Reply all
Reply to author
Forward
0 new messages