Add PulsePattern.plot_grid()

[philsmt]

This is the second part after #499 to add basic visualization for any pulse pattern, here inspired by the common online diagnostics already in Karabo with some tweaks for better offline compatibility.

This is meant to only cover the base implementation in PulsePattern, with advanced implementations like PumpProbePulses customizing this with their specialized data.

[codecov]

Codecov Report

❌ Patch coverage is 68.00000% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.66%. Comparing base (0b977c6) to head (8494848).

Files with missing lines	Patch %	Lines
src/extra/components/pulses.py	68.00%	40 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #504      +/-   ##
==========================================
- Coverage   73.67%   73.66%   -0.02%     
==========================================
  Files          35       35              
  Lines        6831     6956     +125     
==========================================
+ Hits         5033     5124      +91     
- Misses       1798     1832      +34     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[takluyver]

A couple of questions before looking at any of the code:

• What do the darker and lighter colours mean in the example? Is this to do with what fraction of the trains have that pulse?
• What does it look like for PumpProbePulses? (As this is something we use a lot)
• Is it worth trying to label some of the pulses with a number within the plot, to make them easier to identify than tracing across to the axes and doing mental arithmetic? Similar to what we do in Scan.plot() for scan steps.
  • I suspect the answer might well be that it's too hard to do this in a general way that works for any pulse pattern, but maybe you have a better idea how to do it than me.
• If we do have to do mental arithmetic from the axes, 56 squares wide doesn't seem ideal. Can the rows be either a power of 2 or of 10?

[takluyver]

Also, for a sequence split over multiple rows, I might prefer the first row at the top rather than the bottom. But if you've done it this way to match the Karabo widget (I haven't really used that), consistency is a good argument.

[philsmt]

• What do the darker and lighter colours mean in the example? Is this to do with what fraction of the trains have that pulse?

Apologies, I silently expected context from the earlier MR last week and missed it in the description. Transparency (technically color, though I chose the colormap to be roughly equivalent) is used to indicate variable patterns, with the value weighted by the frequency of occurence.

For a constant pattern, you will only see one level. In the example, we're looking at alternating trains of all pulses pumped vs FEL for only the first two. But there are various forms of rotating FEL pulse patterns in use at other instruments the plot must be able to handle.

• What does it look like for PumpProbePulses? (As this is something we use a lot)

As mentioned above, this is meant as a base implementation and thus treats all pulses the same, whether FEL-only, PPL-only, or pumped. In a next step, I would want to customatize this method in PumpProbePulses to behave similar to the Karabo widget (see below) with annotations for pumped and unpumped pulses.

• Is it worth trying to label some of the pulses with a number within the plot, to make them easier to identify than tracing across to the axes and doing mental arithmetic? Similar to what we do in Scan.plot() for scan steps.

Interesting idea, yes. It would somewhat collide with the point above 🙃

• If we do have to do mental arithmetic from the axes, 56 squares wide doesn't seem ideal. Can the rows be either a power of 2 or of 10?

Ah, that sounds like a good idea. Generally I would imagine this plot is mostly used to get a mental image, like done for the original it is modelled after in Karabo:

[philsmt]

Also, for a sequence split over multiple rows, I might prefer the first row at the top rather than the bottom. But if you've done it this way to match the Karabo widget (I haven't really used that), consistency is a good argument.

Therein lies the problem. The SCS/HED version (screenshot above) is top to bottom, the SPB version from LFF is bottom to top. Generally I would gravitate towards the latter, as from a scientist perspective 2D plots continue to have their origin in the lower left.

[takluyver]

Thanks!

For plots with two separate axes I think the origin at the bottom makes sense, but where it's really one long sequence that we're chopping up ~arbitrarily into multiple rows, I think a page of text is a better model. So if we've already got a mixture in Karabo, I'd lean towards following the SCSH/HED version.

[philsmt]

The comparison to Karabo can be distracting though, as these plots will rather appear in notebooks. I would certainly be confused by any plot, 2D or otherwise, to go top to bottom there. This is consistent with the 2D plotting methods in matplotlib like .pcolor[mesh]() and .hist2d. There is .imshow() going the opposite way, but AFAIK this is exactly justified with a technica perspective on rendering images rather than data.

Regarding your earlier point about counting from left to right: Maybe instead of a homogeneous grid, we could go for different grid styles of major/minor ticks to ease counting.

[philsmt]

After the generalizations we made in #499 and some more discussion with Fady, I decided to also make a generic version of that in PulsePattern. Both kind of plots serve different purposes depending on the question you're asking. This one is useful for the conceptual pattern a single instrument and is nice for annotations like pumped/unpumped. The line version gives a better overview and can show any number of patterns side-by-side without visual issues.

Therefore I renamed this method .plot_grid(), with the other version potentially being .plot_lines() or something else.

[tmichela]

+1 for top to bottom. My rational is that this isn't a plot, rather a representation of organized (monotonically increasing indices) data, and it's much natural for me to read left>right, top>bottom.

[philsmt]

I've asked around some more and did not find strong opinions to keep the origin in the lower left.

Improvements based on feedback so far:

• Inverted Y by default
• Added custom start/stop argument, e.g. to cover full RF window
• Pick columns as a multiple of 10*
• Added a grid in X/Y
• More efficient pattern finding

* After playing around with ticks, I noticed even powers of two don't make it particularly intuitive to count. It is fine-ish for X, but then adding the number to Y is cumbersome. This became much better by going to multiples of 10, as now Y's ticks are generally also multiples of 10 and the X-axis runs up to num_cols-1.

[philsmt]

My rational is that this isn't a plot, rather a representation of organized (monotonically increasing indices) data, and it's much natural for me to read left>right, top>bottom.

Shall we rename it then to something like show_grid() or display_grid()?

[philsmt]

Given some time in the DOC, I generalized the plot code now to visualize three pulse patterns similar to the example above. With this, it was trivial to add a custom version to PumpProbePulses:

[takluyver]

Nice, thanks 👍

I was also wondering about suggesting a rename. 'inspect' could be another option, like EXtra-geom. But we have various 'plot' methods like .plot_missing_data and Scan.plot(), so there's an advantage in using that for graphical representations of data in general, even if it's not technically quite right. 🤷

[philsmt]

I think I like inspect(). It is quite nicely fowards comaptible to whatever represents a given component best, where plot() just sounds ambiguous.

We could also introduce this for other components like Scan while keeping the old plot() method around for backwards compatibility.