Support angularly replicated 'symmetric' slicers

[jellybean2004]

Description

This PR introduces symmetry-linked wedge and sector slicers for 2D data.

Users can now specify a number of slicers (N), which are automatically generated as congruent slicers, evenly distributed in angle around the plot centre. One of these will be the 'master' slicer, which is the only directly modifiable slicer, and the others will change automatically based on this.

This also adds the documentation for #3758 and #3806

Fixes #3807 #3794 #3802

To add symmetric slicers:

• Create a 2D plot
• Right click on plot > Slicers > Slicer Parameters
• You will find Symmetric Slicers at the bottom of the first tab
• Select slicer and enter the number of slicers
• Slicers will appear on the plot, with their stacked 1D plot
• Adjust the master slicer to control the whole group.

Symmetric sector slicers, N=3

Symmetric wedge slicer, N=3

## How Has This Been Tested?

Manually tested functionality using 2D data. Verified:

• correct angular replication of slicers
• real-time synchronization when modifying the master slicer
• correct generation of stacked 1D outputs

Review Checklist:

[if using the editor, use [x] in place of [ ] to check a box]

Documentation (check at least one)

• There is nothing that needs documenting
• Documentation changes are in this PR
• There is an issue open for the documentation (link?)

Installers

• There is a chance this will affect the installers, if so
  • Windows installer (GH artifact) has been tested (installed and worked)
  • MacOSX installer (GH artifact) has been tested (installed and worked)
  • Wheels installer (GH artifact) has been tested (installed and worked)

Licensing (untick if necessary)

• The introduced changes comply with SasView license (BSD 3-Clause)

[pmneves7]

I have played around with this on my macbook. The symmetric slicers seem to be working as intended, nice work!

Notes:
I can produce any number of sector or wedge slicers and interactively update them all based on the master slicer. It seems to work as intended. I have not experienced a crash or freeze.

Comments:
It can be a bit slow/laggy, sometimes taking a noticeable fraction of a second to update when I have six slicers. At 20 slicers it takes several seconds to update. This doesn't necessarily have to slow down the pull request, but I am surprised that it is this slow, given that none of the underlying calculations should be that intensive on the CPU. Do you know why this is the case?

Also, do we want to make a warning window every time someone wants to make more than a few slicers? It could be annoying to always be clicking "ok" whenever I want to make a symmetric set of slicers. Maybe it is better to just have a little warning in the "Slice Parameters" GUI that adding more than 4 slicers could be slow.

Future Work:
For hard matter SANS, the next steps (beyond the scope of this pull request) are to allow the user to (1) add together many 2D plots to make one new 2D plot, then do any slicer operation on the summed 2D plot, and (2) allow users to take a series of 2D plots and then plot the sum of a sector as a function of some metadata parameter, creating a 1D dataset. You should also be able to do this with multi-slicers, either summing all the slicer sums together, or plotting each as its own 1D dataset on the same plot.

[jellybean2004]

@smk78, I added docs for the wedge slicers as well as screenshots for each slicer as you stated in your issue. Please let me know if this is all okay.

[pmneves7]

Comments: It can be a bit slow/laggy, sometimes taking a noticeable fraction of a second to update when I have six slicers. At 20 slicers it takes several seconds to update. This doesn't necessarily have to slow down the pull request, but I am surprised that it is this slow, given that none of the underlying calculations should be that intensive on the CPU. Do you know why this is the case?

Would it be faster if, above a certain number of slicers, you can only update via the GUI interface? i.e., is the major slowdown happening in the interactive plot elements, the rendering, or the calculations? If it is the interactive plot elements slowing things down, a GUI-only control may be better.

[smk78]

@smk78, I added docs for the wedge slicers as well as screenshots for each slicer as you stated in your issue. Please let me know if this is all okay.

Read the documentation generated by CI build #5326 . Looks good to me.

[jellybean2004]

Properly rebased to main, removing duplicate commits from the merged parent branch.

[DrPaulSharp]

I've a few suggestions on the code here, I'll leave the functionality testing to @butlerpd as discussed.

[JoGaudet]

I have tested the symmetric slicer option on a PC. The features seem to work as intended for both wedges and sector cuts. I have two suggestions:

1. I believe one should have the option to fold the data or not.

2. Incorporating Box interactors would be very useful. For this, one would build symmetric box interactors with various X and Y interactors. One would need to constraint each Box interactors to (qx,qy)=(0,0) in this case. Does that sound reasonable?

[jellybean2004]

I have tested the symmetric slicer option on a PC. The features seem to work as intended for both wedges and sector cuts. I have two suggestions:

1. I believe one should have the option to fold the data or not.
2. Incorporating Box interactors would be very useful. For this, one would build symmetric box interactors with various X and Y interactors. One would need to constraint each Box interactors to (qx,qy)=(0,0) in this case. Does that sound reasonable?

Hi @JoGaudet. Thank you for your review.

RE your suggestions:

• The fold option you mentioned is currently only visible for the box slicer on main, and seems to have disappeared for the sector slicer. @butlerpd and I led a search party for the missing checkbox a while ago, but we could not find it. If this issue gets resolved, the fold option should appear on the symmetric slicer as well, because the symmetric slicers simply inherit properties from the basic slicer.

• I am finding it a bit tough to visualise how you suggest the box slicers can be implemented as a symmetric slicer. Could you maybe provide a rough sketch of how you would like them overlaid? That would be very helpful for me to map out the implementation.

[DrPaulSharp]

@JoGaudet Thanks for pointing out that the fold option had been removed for sector slicers, it has been reinstated in #3878.

[JoGaudet]

Hi @jellybean2004, I agree that implementing box interactors into symmetric slicers would not be easy, as it would involve creating a new object that can perform averaging that involves both Qx and Qy. What I have in mind is schematized in the following images with real SANS data. Due to the resolution effect when collimation is not dominating, you can see that the shape of Bragg peaks is elongated along Q, while being sharpened along the perpendicular direction. This is very common. In this case, it would be appropriate to use slicers as the one shown in those figures (gray rectangles). For the 4-fold image (on the left), it only requires an independent Box along Qx and another Box along Qy. However, higher symmetry involves a more complicated averaging direction (right image, for example), and is most likely outside the scope of this branch.

Note that I still cannot unfold the different slicers. Let me know when to try.

[jellybean2004]

Hi @JoGaudet, thank you for the detailed visual explanation — the images make it very clear what you have in mind. This type of analysis is exactly what motivated this feature addition. I had a deeper look into adding box slicer support, and here is what I think:

You're right that the 4-fold case (one X-box and one Y-box centred at the origin) is relatively straightforward, as it only requires combining two existing slicer types. However, the general case — symmetric boxes at arbitrary angles, as shown in your higher-symmetry example — would require a new rotated-slab averaging that doesn't currently exist, as well as new interactor geometry to display and interact with the rotated boxes. This represents a substantial amount of new work beyond the scope of this PR, which focuses specifically on angular replication of the existing slicer types (wedge and sector).

In the meantime, for the analysis you're describing, would symmetric wedge slicers be a reasonable substitute? They can target the same elongated peak regions, though along a curved rather than rectangular boundary.

Also, I have incorporated the recent changes with the fold parameter into this. Please let me know if it is all right. Thank you!

[DrPaulSharp]

Why has this pull request been merged without an approving review?

[butlerpd]

More importantly HOW was that possible?! I am assuming the merge was meant as an approval and merge? But I thought we locked github from merging without an approval?

[DrPaulSharp]

Good question @butlerpd. From the following screenshot of #3894 I see that there is an option to bypass the rules: "Merge without waiting for requirements to be met (bypass rules)". I thought this only appeared for the repo admins, if anyone can see it, we need to change the option if possible.

[jellybean2004]

@DrPaulSharp, I am not able to see this option.

[DrPaulSharp]

Righto, thanks @jellybean2004. I'm not certain how it was done in that case then. Any thoughts are welcome, as @butlerpd says, this shouldn't be possible.

[krzywon]

I asked @JoGaudet to do another review, and I guess it was okay by him and went ahead and merged it. I'm wondering, since he had already reviewed (though with a rejection) that GitHub assumed his merge was an implicit approval.

I'm going to submit a different PR with only a single change to a code comment and ask @jellybean2004 to reject it and then see if merging is an option.

[krzywon]

Based on a discussion with @bmaranville, a merge from the command line and then a push will override the branch protections. This is not ideal...

[DrPaulSharp]

Based on a discussion with @bmaranville, a merge from the command line and then a push will override the branch protections. This is not ideal...

Yikes. Yeah, definitely not ideal.

[butlerpd]

Ah ha! so we learned something today. ...Can force all changes to be made through pull requests? does locking a branch still allow pull request merges?

[krzywon]

I found a setting that will hopefully protect the main branch that I've enabled in Settings -> Branches -> main. Admins and Owners will still be able to push to the branch. This setting does not restrict Owners and Admins.

• Restrict who can push to matching branches: Specify people, teams, or apps allowed to push to matching branches. Required status checks will still prevent these people, teams, and apps from merging if the checks fail.
  • Restrict pushes that create matching branches: Only people, teams, or apps allowed to push will be able to create new branches matching this rule.

My one question, which isn't clear, is will this setting allow non admins to merge pull requests?

[JoGaudet]

I confirm that my neophyte self created this mayhem, and the scenario described by @krzywon is correct. I had previously reviewed the functionality and requested a few changes, which were partially implemented and subsequently verified by me. I then agreed that the remaining changes I suggested should not be addressed within this issue. As a result, I tried to find a way to approve the PR without requiring those additional changes—hence the bypass.

[butlerpd]

No worries @JoGaudet -- the point is github should have prevented you from doing this .. precisely so neophytes can't mess it up. I also saw what Jeff is suggesting but my impression was that nobody could then merge? I hope I'm wrong. Nothing like old fashioned experiments to find out I guess.