Fix an error when exporting Sliceviewer pixel cuts from MDHisto workspaces #39071

MohamedAlmaki · 2025-03-17T09:30:31Z

Description of work

This PR fixes a bug encountered when exporting pixel cuts from MDHisto workspaces. The issue arose because the export_pixel_cut_to_workspace is assigned to the MDEvent version in the model. I have added a new function for performing pixel cuts from MDHisto workspaces. Additionally, I have added unit tests for the export pixel cut functions.

Fixes #36084.

To test:

1- Make the workspace md_3D_histo

md_4D = CreateMDWorkspace(Dimensions=4, Extents=[0,2,-1,1,-1.5,1.5,-0.25,0.25], Names="H,K,L,E", Frames='HKL,HKL,HKL,General Frame',Units='r.l.u.,r.l.u.,r.l.u.,meV')
FakeMDEventData(InputWorkspace=md_4D, UniformParams='5e5') # 4D data
FakeMDEventData(InputWorkspace=md_4D, UniformParams='1e5,0.5,1.5,-0.5,0.5,-0.75,0.75,-0.1,0.1') # 4D data

# Add a non-orthogonal UB
expt_info = CreateSampleWorkspace()
md_4D.addExperimentInfo(expt_info)
SetUB(Workspace='md_4D', c=2, gamma=120)

md_3D_histo = BinMD(InputWorkspace='md_4D', AlignedDim0='H,-2,2,100', AlignedDim1='K,-1,1,100', AlignedDim2='L,-1.5,1.5,100')

2- Open md_3D_histo in sliceviewer
3- Select the plot option
4- Press 'x' or 'y' to create a new cut along that axis
5- Check if the cut is correct

Reviewer

Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.

Code Review

Is the code of an acceptable quality?
Does the code conform to the coding standards?
Are the unit tests small and test the class in isolation?
If there is GUI work does it follow the GUI standards?
If there are changes in the release notes then do they describe the changes appropriately?
Do the release notes conform to the release notes guide?

Functional Tests

Do changes function as described? Add comments below that describe the tests performed?
Do the changes handle unexpected situations, e.g. bad input?
Has the relevant (user and developer) documentation been added/updated?

Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers will take care of it.

Gatekeeper

If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.

…workspaces

RichardWaiteSTFC

Thanks for the comprehensive tests!

Testing this manually there does appear to be a problem

The exported pixle cuts just look like straight lines because there's only two bins (you can see this by looking at mtd['md_3D_histo_cut_y].getSingalArray()`

Also if there is an MDEvent workspace e.g. ws.hasOriginalWorkspace() = True then we might want to call the MDEvent function which I think should go an call BinMD on the MDEvent workspace underneath (if it has same number of dimensions - i.e. not in this case as original workspace is 4D)? What do you think?

RichardWaiteSTFC · 2025-03-18T11:16:43Z

So for the MDEvent workspace the pixel cuts exported are cuts - but they don't look much like the cuts in the interface!
It looks to me like the integration limits are wrong - this is screenshot for a cut centred on (H,K) ~ (1,0.6)

It looks like the integration limits for the y-cut (shown in the workspace history on LHS) is -0.385 < K < 1.61 where it should be +/- some small amount around 0.6...

MohamedAlmaki · 2025-03-19T16:42:59Z

So I fixed the plots, the problem was previously the integration limits were just one bin across both two dimensions - I thought that was correct as the name suggests it will extract one pixel, not a 1D slice. In addition, there was a problem when transposing the dimensions.
I have fixed the behaviour and it is now implemented as fixing the intended dimension and integrating along the other dimension with one bin integration limit. Here are the screenshots, let me know if the plots are correct (there is a slight difference as the cursor is sensitive to movement)

RichardWaiteSTFC · 2025-03-19T16:44:55Z

Great thanks! Plots look good, but the only way to know for certain is by checking the workspace history and the integration limits passed. I can check this tomorrow and re-review!

RichardWaiteSTFC · 2025-03-20T17:29:45Z

Tested this - the y-cut looks good, but the x-cut still only has 2 points.
If I look at the call to IntegrateMDHisto for the x-cut for a cut centred on (x,y)=(H,K) ~ (1.3,0.6)

IntegrateMDHistoWorkspace(InputWorkspace='md_3D_histo', 
                          P1Bin='1.27996,0,1.31996', 
                          P2Bin='-1,1', 
                          P3Bin='-3.35276e-08,0.03', 
                          OutputWorkspace='md_3D_histo_cut_x')

and y-cut

IntegrateMDHistoWorkspace(InputWorkspace='md_3D_histo', 
                          P1Bin='1.27996,1.31996', 
                          P2Bin='-1,0,1', 
                          P3Bin='-3.35276e-08,0.03', 
                          OutputWorkspace='md_3D_histo_cut_y')

I think the x-cut (along H) should have P1Bin='-2,0,2', P2Bin=0.6-small,0.6+small

MohamedAlmaki · 2025-03-20T17:58:41Z

Hmmm, are you sure you have pulled the latest changes, here are my results for X cut and they have the same correct values you mentioned:

RichardWaiteSTFC · 2025-03-20T18:02:27Z

Hmm I appear to have it in the log

I'll check again!

RichardWaiteSTFC · 2025-03-20T18:05:58Z

Ah x cut seems to be OK if I export by pressing x-key, but not c-key on keyboard!

MohamedAlmaki · 2025-03-20T18:12:24Z

Ohh now I can reproduce it, seems a problem with c key, I will fix it

MohamedAlmaki · 2025-03-21T15:50:41Z

Exporting both cuts using C key should work well now

RichardWaiteSTFC

Thanks for this - it looks like export_pixel_cut_to_workspace_mdhist and export_pixel_cut_to_workspace_mdevent are basically the same just calling a different function to do the actual cut (which calls different algorithms) is that right? Could we get away with one export_pixel_cut_to_workspace_md which calls a class method which is wrapper around either self.export_cuts_to_workspace_mdhisto or self.export_cuts_to_workspace_mdevent` depending on workspace type?

MohamedAlmaki · 2025-03-21T16:43:39Z

Yeah, there is a lot of duplicate code, I will merge them into one function as you suggest

RichardWaiteSTFC · 2025-03-24T15:06:40Z

Think there's still issues here - if you view (X,Y) = (L,K) the cuts still only contain one bin

Can you reproduce this?

RichardWaiteSTFC

Great, tested on MDEvent and MDHisto with transpose in 4D and 3D and all looking good - thanks for getting this over the line!

jclarkeSTFC

Problem fixed, works fine. Just one question about one of the tests.

jclarkeSTFC · 2025-05-12T09:54:06Z

qt/python/mantidqt/mantidqt/widgets/sliceviewer/test/test_sliceviewer_model.py

+                    mock_extract_roi.assert_called_once_with(self.ws2d_histo, None, None, xpos, xpos, False, "ws2d_histo_cut_x")
+                    self.assertEqual("Cut along Y created: ws2d_histo_cut_x", help_msg)
+
+            for cut_type in ("x", "y"):


Are there also supposed to be calls to assert_call_as_expected with transpose=True? Looks like the logic is there to handle it.

Ohh thanks for spotting that, yeah will add transpose true cases

jclarkeSTFC

Thanks for fixing the test.

…paces (mantidproject#39071) ### Description of work This PR fixes a bug encountered when exporting pixel cuts from MDHisto workspaces. The issue arose because the `export_pixel_cut_to_workspace` is assigned to the MDEvent version in the model. I have added a new function for performing pixel cuts from MDHisto workspaces. Additionally, I have added unit tests for the export pixel cut functions.  Fixes mantidproject#36084.  ### To test: 1- Make the workspace `md_3D_histo` ``` md_4D = CreateMDWorkspace(Dimensions=4, Extents=[0,2,-1,1,-1.5,1.5,-0.25,0.25], Names="H,K,L,E", Frames='HKL,HKL,HKL,General Frame',Units='r.l.u.,r.l.u.,r.l.u.,meV') FakeMDEventData(InputWorkspace=md_4D, UniformParams='5e5') # 4D data FakeMDEventData(InputWorkspace=md_4D, UniformParams='1e5,0.5,1.5,-0.5,0.5,-0.75,0.75,-0.1,0.1') # 4D data # Add a non-orthogonal UB expt_info = CreateSampleWorkspace() md_4D.addExperimentInfo(expt_info) SetUB(Workspace='md_4D', c=2, gamma=120) md_3D_histo = BinMD(InputWorkspace='md_4D', AlignedDim0='H,-2,2,100', AlignedDim1='K,-1,1,100', AlignedDim2='L,-1.5,1.5,100') ``` 2- Open `md_3D_histo` in sliceviewer 3- Select the plot option 4- Press 'x' or 'y' to create a new cut along that axis 5- Check if the cut is correct    --- ### Reviewer Please comment on the points listed below ([full description](http://developer.mantidproject.org/ReviewingAPullRequest.html)). **Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review.** If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed. #### Code Review - Is the code of an acceptable quality? - Does the code conform to the [coding standards](http://developer.mantidproject.org/Standards/)? - Are the unit tests small and test the class in isolation? - If there is GUI work does it follow the [GUI standards](http://developer.mantidproject.org/Standards/GUIStandards.html)? - If there are changes in the release notes then do they describe the changes appropriately? - Do the release notes conform to the [release notes guide](https://developer.mantidproject.org/Standards/ReleaseNotesGuide.html)? #### Functional Tests - Do changes function as described? Add comments below that describe the tests performed? - Do the changes handle unexpected situations, e.g. bad input? - Has the relevant (user and developer) documentation been added/updated? Does everything look good? Mark the review as **Approve**. A member of `@mantidproject/gatekeepers` will take care of it. ### Gatekeeper If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.

MohamedAlmaki added 2 commits March 17, 2025 09:27

Add a new specialized function for exporting pixel cuts from MDHisto …

cf3f37c

…workspaces

Add unit tests for exporting pixel cuts from different workspaces types

b721e93

MohamedAlmaki added Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) Diffraction Issues and pull requests related to diffraction ISIS: Diffraction Issue and pull requests relating to Diffraction at ISIS labels Mar 17, 2025

github-project-automation bot added this to DEVS Mar 17, 2025

github-project-automation bot moved this to Unassigned in DEVS Mar 17, 2025

MohamedAlmaki added this to the Release 6.13 milestone Mar 17, 2025

MohamedAlmaki assigned RichardWaiteSTFC Mar 17, 2025

MohamedAlmaki marked this pull request as ready for review March 17, 2025 11:18

RichardWaiteSTFC requested changes Mar 18, 2025

View reviewed changes

github-project-automation bot moved this from Unassigned to Waiting response in DEVS Mar 18, 2025

Fix integration limits to peform 1D cuts

85d3fe3

Fix X cuts when using C key

cf46f83

RichardWaiteSTFC reviewed Mar 21, 2025

View reviewed changes

MohamedAlmaki added 2 commits March 21, 2025 16:57

Merge export MD cuts into one function

9401c19

Add missing C condition for mdevent testcase

5771804

Fix transpose and bin width issues

61f89b3

RichardWaiteSTFC previously approved these changes May 9, 2025

View reviewed changes

github-project-automation bot moved this from Waiting response to Approved in DEVS May 9, 2025

jclarkeSTFC self-assigned this May 12, 2025

jclarkeSTFC requested changes May 12, 2025

View reviewed changes

github-project-automation bot moved this from Approved to Waiting response in DEVS May 12, 2025

Add missing test cases for matrix workspace pixel cut unit test

457cd02

MohamedAlmaki dismissed RichardWaiteSTFC’s stale review via 457cd02 May 13, 2025 13:21

jclarkeSTFC approved these changes May 13, 2025

View reviewed changes

github-project-automation bot moved this from Waiting response to Approved in DEVS May 13, 2025

jclarkeSTFC enabled auto-merge (squash) May 13, 2025 13:41

jclarkeSTFC merged commit 7fc77c8 into main May 13, 2025
10 checks passed

jclarkeSTFC deleted the 36084_fix_mdhisto branch May 13, 2025 21:14

github-project-automation bot moved this from Approved to Merged in DEVS May 13, 2025

peterfpeterson mentioned this pull request May 14, 2025

Fix an error when exporting Sliceviewer pixel cuts from MDHisto workspaces #39323

Merged

Fix an error when exporting Sliceviewer pixel cuts from MDHisto workspaces #39071

Fix an error when exporting Sliceviewer pixel cuts from MDHisto workspaces #39071

Uh oh!

Conversation

MohamedAlmaki commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of work

To test:

Reviewer

Code Review

Functional Tests

Gatekeeper

Uh oh!

RichardWaiteSTFC left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RichardWaiteSTFC commented Mar 18, 2025

Uh oh!

MohamedAlmaki commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RichardWaiteSTFC commented Mar 19, 2025

Uh oh!

RichardWaiteSTFC commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MohamedAlmaki commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RichardWaiteSTFC commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RichardWaiteSTFC commented Mar 20, 2025

Uh oh!

MohamedAlmaki commented Mar 20, 2025

Uh oh!

MohamedAlmaki commented Mar 21, 2025

Uh oh!

RichardWaiteSTFC left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MohamedAlmaki commented Mar 21, 2025

Uh oh!

RichardWaiteSTFC commented Mar 24, 2025

Uh oh!

RichardWaiteSTFC left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jclarkeSTFC left a comment

Choose a reason for hiding this comment

Uh oh!

jclarkeSTFC May 12, 2025

Choose a reason for hiding this comment

Uh oh!

MohamedAlmaki May 12, 2025

Choose a reason for hiding this comment

Uh oh!

jclarkeSTFC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MohamedAlmaki commented Mar 17, 2025 •

edited

Loading

RichardWaiteSTFC left a comment •

edited

Loading

MohamedAlmaki commented Mar 19, 2025 •

edited

Loading

RichardWaiteSTFC commented Mar 20, 2025 •

edited

Loading

MohamedAlmaki commented Mar 20, 2025 •

edited

Loading

RichardWaiteSTFC commented Mar 20, 2025 •

edited

Loading

RichardWaiteSTFC left a comment •

edited

Loading

RichardWaiteSTFC left a comment •

edited

Loading