PulsePattern component

[philsmt]

Finally, this is the first version of the PulseTiming successor. I decided to split the original component into multiple pieces for a cleaner interface and add further functionality to each. The entire suite is currently planned to consist of:

• PulsePattern to interact with FEL pulses (FEL-only or pump-probe experiments with aligned patterns)
• PulsePatternPPL to specifically look for PPL pulses only
• TODO: PumpProbePattern to combine FEL and PPL for non-aligned patterns
• TODO OpticalLaserDelay combining pulse pattern with BAM corrections and intentional motor-based delay to yield a single delay number for each pulse

The public interface implementation for PulsePattern and PulsePatternPPL is actually identical and in a shared base class _PulsePattern. The reason for this naming is that I wanted to call the version for FEL pulses only PulsePattern, since it will cover the majority of use cases for both FEL-only and pump-probe experiments. This PR only contains the core functionality of PulsePattern for time reasons, the entire interface is planned to consist of:

• select_trains(trains)
• get_pulse_counts(labelled=True): Number of pulses per train as pd.Series or ndarray
• get_pulse_ids(labelled=True): Pulse IDs from bunch pattern table as pd.Series or ndarray
• get_pulse_id_row(): Fast version of get_pulse_ids reading only the first bunch pattern table
• get_pulse_index(with_pulse_id=True): pd.MultiIndex with trains and pulses (by ID or number) to label data
• search_pulse_patterns(with_train_id=True): Search contiguous regions of identical pulse pattern with train slices by ID or index
• trains(): Iterate over (train_id, pulse_ids)
• TODO label_pulse_data(data, train_ids=None, with_pulse_id=True): Attach multi-level index to existing data, say when you loaded pulse-resolved data via KeyData

I'm not really happy with the with_pulse_id/with_train_id flag on some of these function. I think I'd like it to be there so fossils like me can continue using unlabelled ndarray, but the name seems clunky. Any ideas?

[JamesWrigley]

(full disclosure: not an expert with the pulse pattern stuff)

About the with_pulse_id/with_train_id thing, I can't think of anything particularly clean... Best thing I came up with is something like:

get_pulse_index(slice_by="idx") # or "pulse"
search_pulse_patterns(slice_by="train") # or "idx"

I originally thought of index instead of idx, but that would've felt a bit weird with a function already named get_pulse_index().

[JamesWrigley]

BTW, this won't be affected by https://redmine.xfel.eu/issues/151949, right? At least, as far as I can see it doesn't assume that there are 2700 entries in the bunch pattern table.

[philsmt]

BTW, this won't be affected by https://redmine.xfel.eu/issues/151949, right? At least, as far as I can see it doesn't assume that there are 2700 entries in the bunch pattern table.

Yes, thanks to Python being a sane language we usually never assumed an actual length of the bunch pattern table in our code. It'll just work

[philsmt]

I have been thinking more about the naming, and concluded that using the generic word PulsePattern for FEL-pulses specifically may work out, but gets confusing when comparing to the various implementations including optical lasers. If FEL and PPL pulses align it's awesome and obvious, but if not the choice becomes awkward.

So thinking forward to what I want to add next, I would change names to:

• _PulsePattern -> PulsePattern
• PulsePattern -> FelPulses (as it is explicitly looking for FEL pulses and nothing else)
• PulsePatternPPL -> OpticalLaserPulses
• Upcoming class PumpProbePulses to combine both also for asymmetric patterns

[philsmt]

I rebuild the tests now on top of extra_data.tests.mockdata in 57ab01

It was really easy thanks to the existing framework in EXtra-data, and now I have much more confidence in the tests. Naturally there are components that may not need it.

[philsmt]

For reference, we decided to remove the picking-the-output-coords arguments for now and re-add them later in a better form. Also rebased now on master.

[JamesWrigley]

IANAE, but LGTM!

[philsmt]

Thanks for the review!