Skip to content

Add feature flag to read timestamped marginalized stats.

Timothy Davies requested to merge tdv-read-all-stats into spiir-O4-EW-development

Goal

See updated docs in FEATURE_FLAG.md. In short, this works with !222 (merged) (and !220 (closed)) to read recorded marginalized stats.

It should allow us to use up-to-date stats throughout an offline run, as though we were doing an online run.

The motivations are:

  1. It could be important for offline analysis.
  2. It may be necessary to reliably test best-far.

Changes

I refactored a few steps in cohfar_assignfar.c, then added functions to use the new feature flags.

The base case, where you find a timestamp exactly matching the current timestamp, is pretty straightforward. The fallback to find the most recent timestamp was a little verbose, and needed a lot of string copying & freeing.

Tests

I've done some tests both dumping & reading stats, with the exact same start times, and the same cohfar-assignfar-refresh-interval as --finalsink-fapupdater-interval.

I'm also running with export GST_DEBUG=cohfar_assignfar:6.

Running with no offset, the timestamps for dumping and reading exactly match up (in the new version, had to turn a < into <= to match the two intervals), and I can see logs like the following:

0:01:00.257421556 935402      0x2013680 LOG         cohfar_assignfar cohfar/cohfar_assignfar.c:243:get_new_stats_filepaths:<cohfarassignfar3> Loading marginalized stats file '(null)' for timestamp '1186020018000000000' and index '0'.
...
0:05:12.721039664 935402      0x2013c00 LOG         cohfar_assignfar cohfar/cohfar_assignfar.c:243:get_new_stats_filepaths:<cohfarassignfar0> Loading marginalized stats file '/fred/oz016/tdavies/projects/testing/bg/short/spiir_1_1/short_snapshot/dump-all-stats/warp-stats-with-block/1186021218/019_marginalized_stats_2h.xml.gz' for timestamp '1186021218000000000' and index '2'.

Which indicates that it's working.

Running with an offset, I see logs like the following:

0:02:47.641472619 1441625      0x3029800 LOG         cohfar_assignfar cohfar/cohfar_assignfar.c:243:get_new_stats_filepaths:<cohfarassignfar2> Loading marginalized stats file '(null)' for timestamp '1186020317000000000' and index '0'.
...
0:02:48.907716413 1441625      0x3029630 LOG         cohfar_assignfar cohfar/cohfar_assignfar.c:243:get_new_stats_filepaths:<cohfarassignfar3> Loading marginalized stats file '/fred/oz016/tdavies/projects/testing/bg/short/spiir_1_1/short_snapshot/dump-all-stats/warp-stats-with-block/1186020318/019_marginalized_stats_1d.xml.gz' for timestamp '1186020318000000000' and index '1'.
...
0:04:07.770294510 1441625      0x30299e0 LOG         cohfar_assignfar cohfar/cohfar_assignfar.c:243:get_new_stats_filepaths:<cohfarassignfar0> Loading marginalized stats file '/fred/oz016/tdavies/projects/testing/bg/short/spiir_1_1/short_snapshot/dump-all-stats/warp-stats-with-block/1186020618/019_marginalized_stats_2w.xml.gz' for timestamp '1186020628000000000' and index '0'.

Note this run is loading stats later than when the bg run dumped stats, so it's finding a directory with an earlier timestamp (but still more recent than the last).

The first log has no corresponding file, so it keeps going (there's one of these each second until the first load, when running with level 6 gst_debug).

The first load eventually succeeds (when it matches the timestamp).

The final log isn't synced up due to the offset.

So both modes seem to be working.

Review

These flags are only meant for offline tests. I've tried to make the code robust, but it might be hard to follow. Open to any refactor suggestions.

Edited by Timothy Davies

Merge request reports