-
Notifications
You must be signed in to change notification settings - Fork 128
Allow Goniometer to save Rotation Matrix directly to nexus file #39490
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
Conversation
I think the issue is labelled v6.13 so this needs to be pointed at |
I think this could be pushed down the line to 6.14 as not technically a regression but since it is a bug that is much more accessible due to the change to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, changes and tests look great. Test instructions worked as expected and sample shapes correctly set. I agree this deserves a release note as it preserves some precision on the saved/loaded rotation. Just one really nit-picking comment, apologies I had to find something!
file->readData("rotation_matrix", matrixData); | ||
DblMatrix rotMat(matrixData); | ||
setR(rotMat); | ||
initFromR = true; // set as initFromR to prevent overwrite on R recalc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very minor -but I think the naming convention for such member variable (if that is the right term?) is m_initFromR
- obviously this wasn't introduced in this PR (apologies for missing it before), but it helps to make the scope of the variable clear!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for that! Looks good to me!
Description of work
Summary of work
When
saveNexus
is called by Goniometer it now saves a flattened copy of the rotation matrixFixes #39487
Further detail of work
Now the rotation matrix can be directly saved and loaded, which has been identified as an issue because of the new functionality to set goniometer matrices directly: #39219 (comment) but may have also been causing issues in user scripts where goniometer has been set with
setR
or similarTo test:
run script:
Also worth checking that the sample is orientated correctly on all four workspaces (right click on ws,
Show Sample Shape
)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
Functional Tests
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.