Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes and unit tests for AlignAndFocusPowder Refs #23888 #24409

Merged
2 commits merged into from
Jan 7, 2019

Conversation

marshallmcdonnell
Copy link
Contributor

Description of work.
Adds unit test for AlignAndFocusPowder with bug fixes found during the test development.

Bug fixes:

  1. Non-event workspaces did not clone the input workspace, thus an empty workspace was returned
  2. Calling AlignAndFocusPowder with the GroupingFilename it will extract the grouping workspace without applying the mask and pass this to the child algorithm DiffractionFocussing with the GroupingWorkspace argument. Yet if you call DiffractionFocussing with the GroupingFileName argument, it will go ahead and apply the mask. Thus, to make AlignAndFocusPowder with the GroupingFilename work the same way as DiffractionFocussing with the GroupingFileName argument, the mask is now applied to the grouping workspace passed to DiffractionFocussing as the GroupingWorkspace argument.

To test:

  1. Build branch
  2. Build the WorkflowAlgorithmsTest tests
  3. Run the AlignAndFocusPowderTest test suite

Example script for step 2 and 3:
ninja && ninja WorkflowAlgorithmsTest && ctest -R AlignAndFocusPowderTest

NOTE: If it helps, for manually setting up the tests on HRPD test files in the ExternalDataStore, the following python script helped me plot results to figure out when I encountered bugs. (has .txt file extension so GitHub would let me upload)
alignfocuspowder_testing.txt

Fixes #8929, fixes #17840, and fixes #23888

Refs #17841, #19105, #19109, #22602, #23869, and #24386


Reviewer

Please comment on the following (full description):

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 are changes in the release notes then do they describe the changes appropriately?
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.

@marshallmcdonnell marshallmcdonnell added Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) Quality: Coverity Framework Issues and pull requests related to components in the Framework labels Jan 4, 2019
@marshallmcdonnell marshallmcdonnell added this to the Release 3.14 (4.0?) milestone Jan 4, 2019
Copy link
Member

@peterfpeterson peterfpeterson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall a great improvement as demonstrated by the bugs found and fixed. The changes requested are code style/convention.

int numEvents = 10000) {
inputWS = "eventWS";

CreateSampleWorkspace createSampleAlg;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be a good idea (follow-on PR) to make this the default functionality for CreateSampleWorkspace(Function='Powder Diffraction', ...) as having the instrument centered on two_theta=90 is fairly common.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue created #24411

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues and pull requests that are regressions or would be considered a bug by users (e.g. crashing) Framework Issues and pull requests related to components in the Framework
Projects
None yet
3 participants