Update ChainReader to use trajectory time or calculated frame, fail cleanly

[dotsdl]

Subtask for #252:

Update ChainReader

• Let the ChainReader try to use the trajectory time or the calculated time.
• But if this fails (e.g. time for step N+1 is less than time for step N) use time_offset with some heuristic guesses to appropriately glue trajectories together.

[dotsdl]

Working on this one this week. Should be easier given the work that's been done by @richardjgowers in #306 and #307.

[orbeckst]

Is anything else blocking this one anymore?

It would be good to have this within Milestone:0.11 as part of the trajectory overhaul.

[dotsdl]

Agreed. Hoping to submit PR today.

[richardjgowers]

@dotsdl I'm going to bump this to 0.12 unless I hear screams otherwise

[dotsdl]

Agreed. I am working to overhaul the ChainReader, but it will take more time and thought. Anyhow, as written the ChainReader does work with 0-based frames, so at least the changes for this release don't break it.

[orbeckst]

We can put it in a 0.11.1

On 31 Aug, 2015, at 10:22, David Dotson wrote:

Agreed. I am working to overhaul the ChainReader, but it will take more time and thought. Anyhow, as written the ChainReader does work with 0-based frames, so at least the changes for this release don't break it.

Oliver Beckstein * orbeckst@gmx.net
skype: orbeckst * orbeckst@gmail.com

[orbeckst]

How much of this issue was done in #523? Should me move this to 0.14?

[orbeckst]

@mnmelo can you comment on how much of this issue #309 (Update ChainReader to use trajectory time or calculated frame, fail cleanly) was accomplished with your recent PR #523?
It always makes me happy to close issues...

[mnmelo]

Sorry, I hadn't noticed the previous question had been directed at me.

With #523 we can now always tell a time from the ChainReader. However, this time is now computed solely from the dts of the underlying Readers and assumes we start at t=0.

In short, we are ignoring time information from the trajectories, and therefore currently can't correctly read times when:

• the first sub-trajectory doesn't start at time=0;
• there is a time gap (different from dt) between sub trajectories;
• there's a varying dt along a trajectory.

The case of times in a sub-trajectory being smaller than times in a previous sub-trajectory isn't handled, but also doesn't fail.

From here, I think if we take into account the starting times of each sub-trajectory we'll already be on a sensible path. To cover all aspects maybe then implement a ChainReader flag that lets the user specify if they want a raw, monotonic, or gapless time reporting (raw: read time directly from the subreader; monotonic: compute time from initial frames and dts, but increase the time of sub-trajectories if it is smaller than previous times; gapless: like monotonic but disallow gaps larger than the sub-trajectories' dts.)

[orbeckst]

This still seems like a sizable chunk of work so I am bumping it to 0.14 1.0 (or somewhere thereabout).

It would be good to come at least to a conclusion what we want to implement here. It might be easier if we stored frame times (ir they can be read from trajectory) with the persistent offsets because we could then do searching and all kind of array operations on the time arrays.