Add PumpProbePulses for combined FEL/PPL pulse patterns

[philsmt]

The third component in the mix for pump-probe experiments 🔦

• Combines FEL and PPL pulses into a single linear pattern
• Move PPL pulses to their true location by fixed bunch table position or relative bunch table/pulse distance to first FEL pulse
• Extends the MultiIndex by two boolean fields fel and ppl to distinguish between respective shots

[philsmt]

Re-implemented this now on top of the new PulsePattern based on Pulse IDs, which made the code much simpler. It still makes use of the in8-view trick, but only ofr the purpose of pattern comparisons.

Ready for review now with tests.

[JamesWrigley]

Mostly being very nitpicky about typos 🔍 This should also be documented in components.md and have a changelog entry.

[philsmt]

Thanks! Somehow I'm still forgetting to document the changes.

[JamesWrigley]

I'd add an internal comment about super() (#24 (comment)), but otherwise LGTM!

[philsmt]

I hope I didn't get carried away in 24400ea 😁

[JamesWrigley]

A thing of beauty 😆

[JamesWrigley]

I sneakily used this for MID's experiment. It's pretty darn sweet, I found exactly what I was looking for in two lines of code 😋

Also found some edge-cases:

• For some runs (e.g. p4451, run 78) there were more laser pulses configured than FEL pulses, which was slightly confusing when plotting the pulse pattern because there would be more points than pulses:
  

  What I'm typically looking for is the combination of FEL/PPL pulses, so what if peek_pulse_ids() had something like a only_with_fel=True argument to drop PPL pulses without an FEL pulse?

• PumpProbePulses._get_ppl_offset() fails if the pulse offset is specified and there's only one pulse (e.g. p4451, run 80), because on line 923 it tries to look at the second pulse. I worked around it with:

       elif self._pulse_offset is not None:
  -         return fel[0] + int((fel[1] - fel[0]) * self._pulse_offset)
  +        if len(fel) > 1:
  +            return fel[0] + int((fel[1] - fel[0]) * self._pulse_offset)
  +        else:
  +            return fel[0]

[philsmt]

Awesome, happy to hear!

• For some runs (e.g. p4451, run 78) there were more laser pulses configured than FEL pulses, which was slightly confusing when plotting the pulse pattern because there would be more points than pulses:
  [img]
  What I'm typically looking for is the combination of FEL/PPL pulses, so what if peek_pulse_ids() had something like a
  only_with_fel=True argument to drop PPL pulses without an FEL pulse?

Would you have the same result with pp_pulses.peek_pulse_ids()[:, True], i.e. all pulses with fel=True?

• PumpProbePulses._get_ppl_offset() fails if the pulse offset is specified and there's only one pulse (e.g. p4451, run 80), because on line 923 it tries to look at the second pulse. I worked around it with:

       elif self._pulse_offset is not None:
  -         return fel[0] + int((fel[1] - fel[0]) * self._pulse_offset)
  +        if len(fel) > 1:
  +            return fel[0] + int((fel[1] - fel[0]) * self._pulse_offset)
  +        else:
  +            return fel[0]

Hmm, but that moves the PPL pulses for pulse_offset != 0 to a different position then expected. The offset cannot be inferred in units of pulses if there is always only a single pulse (for intermittent cases there's extrapolate=True), unless we'd add a manual override for repetition rate in such a case.

[JamesWrigley]

Would you have the same result with pp_pulses.peek_pulse_ids()[:, True], i.e. all pulses with fel=True?

Ah yes that's perfect, NVM then. I didn't realize you could index multiindex's like that.

Hmm, but that moves the PPL pulses for pulse_offset != 0 to a different position then expected. The offset cannot be inferred in units of pulses if there is always only a single pulse (for intermittent cases there's extrapolate=True), unless we'd add a manual override for repetition rate in such a case.

Hmm I see, indeed that didn't matter last week because the pulse offset was always 0. What if the constructor checked for there only being one pulse and pulse_offset != 0 and threw an error if so? The point being to warn the user immediately.

[philsmt]

I'll take James' earlier LGTM to finally get this in.