simonsobs / sotodlib

Simons Observatory: Time-Ordered Data processing library.
MIT License
16 stars 19 forks source link

Add `timestamps_linearized` to g3tsmurf.load_file #253

Closed msilvafe closed 2 years ago

msilvafe commented 2 years ago

Because of the latency of the smurf processor the unix timestamps have nonuniform spacing as discussed in #127, the smurf header has counter information that is written at the FPGA level and so does not suffer from delays in the smurf processor. The counter can be used to linearize the timestamps. We should implement in the g3tsmurf loader an added axis that corresponds to this linearized timestamps, this could be implemented as:

fsamp, offset = np.polyfit(am.primary["FrameCounter"], am.timestamps, 1)
timestamps_linearized = offset + fsamp*am.primary ["FrameCounter"]

am.wrap("timestamps_linearized", timestamps_linearized, [(0,"samps")])
kmharrington commented 2 years ago

I know @mhasself said he liked that idea but I'm not a massive fan of this being implemented by default. For one thing, when we're running with the unix timing paradigm we're already having to assume our timing is suspect in all cases (we have had to do this in the LATRt). We should never be running with this paradigm in the field and won't be able to reconcile the pointing/HWP angles if we are.

When we get the timing system and better knowledge of timestamps we already have a ready-made place to plug in the new information here.

If it's some sort of utility function you'd like added, the primary header information is inside the AxisManager already so there could be a secondary function that calls this or something.

jlashner commented 2 years ago

I kinda think this should be the default (when there's no timing system) because in general the linearized timestamps are going to be more accurate than the software timestamps, and it'll fix many of gaps like the one below that are caused by frames being held up in the smurf-processor queue while it waits for G3Frames to be compressed and written: image

I end up doing this manually most of the time I load data.

kmharrington commented 2 years ago

Ok right, if it's regularly going to be the best thing when running in the unix paradigm I would just stick this into timestamps and not make a whole new field. The new field will make it annoying to use the right fields in the FFT and filtering code. And this shouldn't mess up our nice timestamps when they exist.

msilvafe commented 2 years ago

I would be even happier if this was just the timestamps field and we get rid of the Unix timestamp entirely

msilvafe commented 2 years ago

And I'm not really sure what you mean expect our timing is "suspect in all cases" with these linearized timestamps & the encoders & Smurf 122.88 MHz clocks locked to a common 10MHz signal your timestamps should have virtually all jitter removed and you're just left with an offset in absolute time between your encoder & Smurf timestamps that could be large & vary from run to run (this bit is what the Smurf timing system should solve). This is described in Matthew's detailed timing document in section "Timing for the SMuRF System". Jack's plot above confirms this and Tomoki & Junya did some tests of this at UTokyo as well described here (especially slides 13-16)

kmharrington commented 2 years ago

That's fair, I am likely over-exaggerating the timing confusion because our chopper can't read in 10 MHz so we're probably more confused/limited by that.

mhasself commented 2 years ago

The only thing that bothers me about "linearized" timestamps is that they're not always the same... it depends on what segment of data you load. E.g. if you load the first half of a file, and separately load the second half, the concatenation of lin. timestamps from those two segments will not be equal to the lin. timestamps from the whole file. But the difference will be tiny, in most cases. If you all can live with that ... then I can live with that.

If we replace .timestamps, can the original timestamps be kept around somehow? When something goes weird with lin. timestamps we'll want to look at those. (This will also be the case with the full Smurf timing system -- we'll want access to the various timestamps to check consistency.)

kmharrington commented 2 years ago

We could add a keyword argument on the load to say if you want the timing linearized or not. We do that for other type things in the main load function as well.

jlashner commented 2 years ago

am.primary['UnixTime'] should still contain the original unix timestamps regardless.