sandialabs / pyGSTi

A python implementation of Gate Set Tomography
http://www.pygsti.info
Apache License 2.0
134 stars 56 forks source link

Per Sequence Detail Hover-Box Improvement #419

Open pcwysoc opened 5 months ago

pcwysoc commented 5 months ago

Is your feature request related to a problem? Please describe. The hover-box labels obscure parts of the per sequence detail plot and it can be difficult to hover over the correct box when it is obscured. This is an ongoing frustration when studying HTML reports in depth and would be really nice to see it fixed at some point.

Describe the solution you'd like

Screenshot 2024-04-10 at 2 15 13 PM
coreyostrove commented 5 months ago

Thanks for the suggestion, @pcwysoc. Jotting down a couple quick notes now while this is fresh in my mind. The unfortunate answer is that this change, as described, isn't possible due to limitations in the available options in plotly for changing the behavior of hover text tooltip boxes (plotly is the graphics library which powers nearly all of the report generation). See the documentation here for more details on supported options: https://plotly.com/python/hover-text-and-formatting/.

While reading through some forum threads on the plotly website related to this, however, someone mentioned that it should be possible in plotly to make the annotations clickable/toggleable in the same way we do for table captions. So that may be an option to consider if we wanted to have an option for changing this behavior.

Alternatively, if this is primarily a navigation issue, do you think it would ameliorate the usability issue if we enabled what Plotly calls in their documentation 'spike lines'? These are demoed here: https://plotly.com/python/hover-text-and-formatting/#spike-lines, for reference. This would be a less-invasive modification to make.

pcwysoc commented 5 months ago

@coreyostrove Bummer. I was worried that might be the case. Spike lines might be a good work around as it is primarily but not exclusively a navigation issue. I'd have to see how that looked though. Another option might be making the background fill ever so slightly opaque which I think is possible.

@kmrudin Any thoughts?

coreyostrove commented 5 months ago

For spike lines there is an interactive demo on the plotly documentation page I linked to, if you want to give that a spin. This may not work in practice, but if the issue is the visibility of the boxes, we could try out adding a 1px border around the squares to bump up the contrast. An adjacent issue I find myself running into these days more and more (in part because my eyesight isn't getting any better with time) is that it can be tricky to inspect the per-sequence details for heavily-reduced experiment designs in which these plots can be quite sparse. In those cases, which often lack an immediately clear structure like the full designs do, it can be hard to spot very light-colored squares, so this may ameliorate that as well. As an alternative to bumping up the background opacity, we could also look into decreasing the opacity of the tooltip box so that you can make out the positions of squares underneath.

coreyostrove commented 5 months ago

Here is my first pass at adding borders to the boxes in the per-sequence detail.

Without FPR: 2px_w_o_fpr

With FPR: 2px_w_fpr

It turns out plotly (somewhat surprisingly) doesn't natively support borders for the boxes in Heatmap plots (which is what is used for generating the per-sequence detail under the hood). I stumbled onto a helpful forum thread (https://community.plotly.com/t/heatmap-cell-border/62413/3) where a user cleverly realized you could (more or less) achieve the same visual effect by superposing two Heatmap plots on top of each other. So that is what I've done here.

I'm fairly happy with this, but of course open to feedback/refinements. One change that might be nice to continue pushing on improved readability of these for FPRed experiment designs would be to add grid lines separating between the different plaquettes, the boundaries of which can get a bit hard to discern with heavily reduced experiment designs.

pcwysoc commented 5 months ago

I really like the look of the borders! Is this on a branch? I'd love to see if this also helps navigate when the hover label covers most of the plaquette (which happens especially with PC GST), although I've seen cases where the hover label covers the entire plaquette.

coreyostrove commented 5 months ago

I've just published a branch with the latest iteration on this. It is called 'feature-report-tweaks'. I've gone through a few more iterations and added some grid lines for better distinguishability between sparse plaquettes, and also tweaked some spacing along the boundaries. Open to feedback and other suggestions.

w_o_fpr_rev2 w_fpr_rev2

coreyostrove commented 5 months ago

By popular request we're going to let democratic processes decide the formatting for the visual updates to the per-sequence detail plots. Below are 6 different variations on the border formatting. Take a look and cast your vote for which you prefer, plurality takes it. To cast your votes add a reaction to this post using the emoji mapping below (you can react by clicking on the smiley face in the bottom left corner of this post). In the event of a tie, I will serve as the tiebreaker.

1: 😄 2: 🎉 3: 😕 4: ❤️ 5: 🚀 6: 👀

Option 1: 2px Border No Gap option_1_2px_border_no_gap

Option 2: 2px Border 1px Gap option_2_2px_border_1px_gap

Option 3: 1px Border No Gap option_3_1px_border_no_gap

Option 4: 1px Border 1 px Gap option_4_1px_border_1px_gap

Option 5: 1.5px Border No Gap option_5_15px_border_no_gap

Option 6: 1.5px Border 1px Gap option_6_15px_border_1px_gap

pcwysoc commented 5 months ago

I think the plots without the gap look blurry and the thick line also reduces visibility. I think a 1-1.5px line and the gap work best.

robinbk commented 5 months ago

I don't know how to do emojis but I cast my vote for Option 3 (😕), with Option 1 in 2nd place.

On Fri, Apr 26, 2024 at 12:38 AM coreyostrove @.***> wrote:

By popular request we're going to let democratic processes decide the formatting for the visual updates to the per-sequence detail plots. Below are 6 different variations on the border formatting. Take a look and cast your vote for which you prefer, plurality takes it. To cast your votes add a reaction to this post using the emoji mapping below (you can react by clicking on the smiley face in the bottom left corner of this post). In the event of a tie, I will serve as the tiebreaker.

1: 😄 2: 🎉 3: 😕 4: ❤️ 5: 🚀 6: 👀

Option 1: 2px Border No Gap option_1_2px_border_no_gap.png (view on web) https://github.com/sandialabs/pyGSTi/assets/78768318/a2476097-ce8e-4cc8-9e25-d89418bdb41d

Option 2: 2px Border 1px Gap option_2_2px_border_1px_gap.png (view on web) https://github.com/sandialabs/pyGSTi/assets/78768318/b2f78b9b-75dd-4df5-980d-4248ab11bbeb

Option 3: 1px Border No Gap option_3_1px_border_no_gap.png (view on web) https://github.com/sandialabs/pyGSTi/assets/78768318/ff5c7f9a-97c6-47c6-b257-33e91adf5d0b

Option 4: 1px Border 1 px Gap option_4_1px_border_1px_gap.png (view on web) https://github.com/sandialabs/pyGSTi/assets/78768318/fcc416d3-8eb8-42d1-8c10-0616e6513012

Option 5: 1.5px Border No Gap option_5_15px_border_no_gap.png (view on web) https://github.com/sandialabs/pyGSTi/assets/78768318/a9163933-ea51-4e04-bc9e-4768d26fafbd

Option 6: 1.5px Border 1px Gap option_6_15px_border_1px_gap.png (view on web) https://github.com/sandialabs/pyGSTi/assets/78768318/00e45557-3c52-4bd1-bb38-3e2f731b8d2c

— Reply to this email directly, view it on GitHub https://github.com/sandialabs/pyGSTi/issues/419#issuecomment-2078615715, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNUZ5JDAKT3LWOZN5I7KELY7HK5DAVCNFSM6AAAAABGBCXJACVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZYGYYTKNZRGU . You are receiving this because you are subscribed to this thread.Message ID: @.***>