beacon-biosignals / TimeSpans.jl

A Julia package that provides a `TimeSpan` type for representing a continuous span between two points in time.
Other
6 stars 2 forks source link

Should we allow zero-duration TimeSpans? #55

Open ssfrr opened 1 year ago

ssfrr commented 1 year ago

I'm guessing this has been discussed and there's a good reason, but I haven't been able to find it.

It seems odd to me that you can't create a zero-duration TimeSpan . It seems to be a consequence of the combination of

Are both those necessarily true? It seems like you could either relax the constraint such that start <= stop, or compute the duration as stop-start-1.

I can think of 3 different semantics you could attach to a TimeSpan:

  1. start and stop mark points on a continuous timeline, and they happen to be quantized to 1ns, so TimeSpan(x, y) represents the span [x,y), where the endpoint is infinitesimally close to y on the timeline. In this interpretation:
    • duration = stop - start
    • TimeSpan(x, y) and TimeSpan(y, z) are contiguous (assumingx < y < z).
    • stop >= start
  2. start and stop are indices referring to the 1ns chunks of time that is occupied by this TimeSpan. This can be further subdivided a. The range is inclusive, (like the julia range 4:6 which represents [4,5,6]). This is explicitly ruled-out in the docs which say it represents "the interval [start, stop)", but isn't an unreasonable semantic, and would imply:
    • duration = stop - start + 1
    • TimeSpan(x, y) and TimeSpan(y+1, z) are contiguous
    • stop >= start-1 (it seems a little weird to represent an empty span with t, t-1 but that's how range does it) b. The range is exclusive, which implies:
    • duration = stop - start
    • TimeSpan(x, y) and TimeSpan(y, z) are contiguous.
    • stop >= start

1 and 2b are in practice the same thing, and are what's implemented. They are internally consistent (e.g.duration(TimeSpan(x, z)) == duration(TimeSpan(x, y)) + duration(TimeSpan(y, z)), and overlaps is false).

Most of this was just me thinking through the details to make sure I understood. I'm still left with the question though of why can't stop == start to represent a zero-length span?

(side note - it makes sense that 1 and 2b would be the same, as 1 is just the limit of 2b as the chunk size goes to 0)

ericphanson commented 1 year ago

Yeah, I believe the intention is left-inclusive, right-inclusive exclusive (i.e. your (1) / 2b). That means that TimeSpan(1,1) doesn't represent the instant of time 1, but rather is an empty interval. So it's not just a zero-length span, it's a span that doesn't contain any time at all. I believe this is disallowed because if it could be created, in practice it would get confused for a timespan containing the instant 1.

ericphanson commented 1 year ago

BTW, AlignedSpans.jl provides a TimeSpans.jl compatible type that gives access to sample-aligned inclusive-inclusive intervals. There you can have an AlignedSpan(sample_rate, 1, 1), but this does not have duration 0, rather it has the duration of 1 sample.

ssfrr commented 1 year ago

I believe the intention is left-inclusive, right-inclusive

I think that should be right-exclusive, i.e. the interval doesn't contain the stop time

That means that TimeSpan(1,1) doesn't represent the instant of time 1, but rather is an empty interval. So it's not just a zero-length span, it's a span that doesn't contain any time at all.

I agree it doesn't represent an instant, but I don't understand the distinction between a zero-length span and a span that doesn't contain any time.

BTW, AlignedSpans.jl provides a TimeSpans.jl compatible type that gives access to sample-aligned inclusive-inclusive intervals. There you can have an AlignedSpan(sample_rate, 1, 1), but this does not have duration 0, rather it has the duration of 1 sample.

Yeah, I just confirmed that:

julia> duration(AlignedSpan(1, 5, 5))
1000000000 nanoseconds

julia> duration(AlignedSpan(1, 5, 6))
2000000000 nanoseconds

julia> duration(AlignedSpan(1, 5, 7))
3000000000 nanoseconds

That seems like a confusingly subtle difference between AlignedSpan behavior and TimeSpan behavior, and I guess implies that it has (1a) semantics?

I still don't see a compelling reason to disallow zero-length TimeSpans where start==stop. It seems like a special case that complicates the definition, and also breaks some nice mathematical properties, mostly that there's no identity element for TimeSpan concatenation. It also means that there's no well-defined answer for "give me the timespan between these two timespans", because if they're contiguous then you'd want to return the zero-length span, but it doesn't exist.

Another way to look at it is that you should be able to define an equivalent type with start and duration fields, and it would seem odd if duration couldn't be 0, right?

jrevels commented 1 year ago

I think that should be right-exclusive, i.e. the interval doesn't contain the stop time

that's right

I still don't see a compelling reason to disallow zero-length TimeSpans where start==stop.

the original reason was kinda what Eric said - if we allowed zero-duration spans, then all generic time-span consuming code would have to properly handle that zero-duration edge case to be properly generic, and we felt it was easier to just disallow it then have consumers possibly shoot themselves in the foot w/ that extra edge case

however, as you call out, not having zero-duration spans can be its own kind of mathematical footgun in some cases

so I'm fine w/ making a change to allow zero-duration spans if folks want (the change should be pretty simple to implement here IIUC), but it'd be a breaking change so would have to version accordingly. PR welcome!

jrevels commented 1 year ago

(changed the title which could be slightly misleading if one doesn't understand the kind of interval a TimeSpan represents)

ericphanson commented 1 year ago

That seems like a confusingly subtle difference between AlignedSpan behavior and TimeSpan behavior, and I guess implies that it has (1a) semantics?

So AlignedSpans exists to provide a way to convert back and forth between sample indices and TimeSpans, and it provides an object AlignedSpan that's backed by sample indices, but fulfills the TimeSpans.jl interface (by supporting start and stop). So I don't think it has different semantics as a TimeSpans.jl timespan. That is start and stop provide inclusive-exclusive semantics, and an AlignedSpan represents a non-empty inclusive-exclusive stretch of time. But it can be constructed from inclusive-inclusive sample index intervals, which is the natural way to work with sample indices (e.g. like Julia UnitRange{Int}s).

ericphanson commented 1 year ago

I agree it doesn't represent an instant, but I don't understand the distinction between a zero-length span and a span that doesn't contain any time.

Well, the inclusive-inclusive interval [1, 1] has duration 0 but is non-empty (contains the time 1), whereas the interval [1,1) is empty (contains no time at all).

ssfrr commented 1 year ago

Well, the inclusive-inclusive interval [1, 1] has duration 0 but is non-empty (contains the time 1), whereas the interval [1,1) is empty (contains no time at all).

Ah, I see what you mean. I think we agree here. Another way to define a timespan [x, y) is the set {t: x <= t < y}, so [x, x) doesn't include x. This definition also nicely works in both discrete and continuous time.

I'm not really convinced by "in practice it (TimeSpan(1,1)) would get confused for a timespan containing the instant 1." I think it's way more confusing if someone who understands half-open intervals has their 0ns interval silently changed to a 1ns one. Are you aware of any concrete examples where that confusion has been a problem?

More pragmatically, would you be good with a PR that allowed zero-length timespans?

I think the AlignedSpans conversation can be split out from this one, and I haven't grokked it enough to have a well-formed opinion yet. I'll read through that package a bit more.

ericphanson commented 1 year ago

I think it's way more confusing if someone who understands half-open intervals has their 0ns interval silently changed to a 1ns one.

Personally I would be open to removing this.

would you be good with a PR that allowed zero-length timespans?

I’m not the main maintainer here (that’s Jarrett) but I would use missing instead to represent a non-existent interval of time. As Jarrett mentioned, allowing empty ones will need special casing everywhere. And I think it’s confusing that TimeSpan(1,1) == TimeSpan(999,999) (as sets, at least), meaning the numbers inside are meaningless when it’s empty. I think storing fake numbers will just lead bugs where someone uses them by accident.