Skip to content

Offset random event times so the first chunk never starts before 0 - #387

Open
DonNabla wants to merge 2 commits into
mainfrom
fix/random-event-times-offset
Open

DonNabla wants to merge 2 commits into
mainfrom
fix/random-event-times-offset

Conversation

@DonNabla

@DonNabla DonNabla commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

What does the code in this PR do / what does it improve?

Fixes the Attempt to create chunk ... with negative start time error raised by strax for some jobs simulated with random event times (fixed_event_spacing=False).

ChunkInput starts the first chunk first_chunk_left (1 ms by default) before the first interaction. With fixed spacing, event times get a 1 s offset, so that margin always fits. With random times there is no offset: times are drawn uniformly from 0, so whenever the first event lands within 1 ms of 0, the first chunk starts below 0. At the default 1 Hz rate that is about 1 job in 1000. The seed is deterministic, so the affected jobs fail identically on every retry.

Can you briefly describe how it works?

The random-times branch now adds the same 1 s offset as the fixed-spacing branch, so the full first_chunk_left margin is kept for every job (nothing is clamped). The fixed-spacing branch is unchanged.

Lineage impact: ChunkInput goes from 0.4.1 to 0.4.2. With random times, every event is shifted by 1 s, so output differs from 0.4.1 by that constant offset. The new version also changes the lineage, and with it the deterministic seeds, of everything downstream, so it is not bit-for-bit comparable with earlier productions. That applies to fixed-spacing simulations too, even though their output is unchanged. No reprocessing of existing simulations is implied; the jobs that already succeeded are valid.

Note: cut_delayed is compared against the full interaction time, event time included, so with random times it now sees the same 1 s offset that fixed spacing already had. With the default (1e18 ns) this makes no difference.

NumPy 2.4 compatibility: NumPy 2.4 removed np.in1d, which broke the latest CI job (54 tests failing with module 'numpy' has no attribute 'in1d') in SecondaryScintillation, S1PhotonPropagation and LineageClustering. The three calls are replaced by np.isin. All three inputs are 1-D, so the result is identical and no plugin versions change.

Can you give a minimal working example (or illustrate with a figure)?

tests/test_random_event_times.py drives file_loader with a small CSV and a random generator that puts every event at the lower edge (t = 0):

  • on main: the first chunk starts at -1000000 ns, so the test fails;
  • with this PR: the first chunk starts at 0.999 s, all interactions fall inside their chunk bounds, and the test passes.

Please include the following if applicable:

  • Update the docstring(s) (none needed)
  • Bump plugin version(s) (ChunkInput 0.4.2)
  • Update the documentation
  • Tests to check the (new) code is working as desired.
  • Does it solve one of the GitHub open issues?

With fixed_event_spacing=False, event times were drawn uniformly from 0,
so an event within first_chunk_left (1 ms) of 0 put the first chunk's
start below 0 and strax raised 'chunk with negative start time'. The
deterministic seed made every retry fail the same way. Add the same 1 s
offset the fixed spacing already uses, and bump ChunkInput to 0.4.2.
@DonNabla
DonNabla requested a review from HenningSE November 2, 2026 08:26
@HenningSE

Copy link
Copy Markdown
Collaborator

Looks good to me! Once we have the checks working we can merge this!

All three call sites pass 1-D arrays, so the result is unchanged.
@DonNabla
DonNabla marked this pull request as ready for review November 3, 2026 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants