Change left/right symmetry in reduction of (mis)orientations to fundamental zone
Description of the change
This PR changes the order of symmetry application in the reduction of misorientations to the misorientation fundamental zone from sym_A * m_AB * sym_B to sym_B * m_AB * sym_A. The new order is in line with the notion of the misorientation from o_A to o_B being given by m_AB = o_B * o_A^-1.
Note that the same algorithm is used to reduce orientations to the Rodrigues fundamental zone, in which case the two symmetries are the symmetry of the sample reference frame, sym_A = C1 and the symmetry of the crystal, say sym_B = Oh.
This change was motivated by several inconsistent results reported lately. This change fixes all these issues:
- https://github.com/pyxem/orix/issues/368#issuecomment-1163212482
- https://github.com/pyxem/orix/issues/434#issuecomment-1492180192
- #437
I had to update all three clustering tutorials. They give comparable results, although I'm unsure of how to solve the mirroring of the clustered Ti orientations in that tutorial. Any help on spotting an error would be appreciated.
The other major change is naming: map_into_symmetry_reduced_zone() -> reduce(), inspired by the attractive naming of scipy.spatial.transforms.Rotation.reduce(). I've deprecated the former method in favor of the new one, with removal in v0.13. Arguably not as easy to understand as the former name, but the notion of reducing a (mis)orientation to its fundamental zone is there. Using this word allows us also to add reduce as an intuitive parameter to various methods if we don't want to apply symmetry operations, instead of having a use_symmetry parameter (similar to MTEX' noSymmetry). We could then rename Miller.in_fundamental_region() to Miller.reduce() etc.
Testing this change required some other tools and changes, which are also included in this PR:
- New class method
Vector3d.get_path()to get vectors along the path between two or more vectors. Useful to plot a bounded region on the stereographic projection, or delineate a path between two vectors. - Doc example showing how to combine rotations, i.e. from left to right. The example is the one in section 4.2.2 in Rowenhorst et al. (2015).
- Changed time of removal of
conventionparameter in to/from_euler() methods from v1.0 to v0.13, because I don't see v1.0 on the horizon. - Make stereographic polar grid use Matplotlib's
grid.alphaconfig value. - Restructure some tests for clarity.
Progress of the PR
- [x] Docstrings for all functions
- [x] Unit tests with pytest for all lines
- [x] Clean code style by running black via pre-commit
- [x] Scrutiny of the change by others, like testing on their own problems. I've reached out to @maclariz, @DorianDepriester and @anderscmathisen in the above mentioned issues for their feedback.
Minimal example of the bug fix or new feature
I suggest to inspect the clustering tutorial notebooks in the docs built from this PR:
For reviewers
- [ ] The PR title is short, concise, and will make sense 1 year later.
- [ ] New functions are imported in corresponding
__init__.py. - [ ] New features, API changes, and deprecations are mentioned in the unreleased
section in
CHANGELOG.rst. - [ ] Contributor(s) are listed correctly in
__credits__inorix/__init__.pyand in.zenodo.json.
kikuchipy's tests pass with this branch. The functionality in kikuchipy relying on reduction to Rodrigues fundamental zone is the uniform sampling of orientations. This seems to be unaffected by this change.
Considering the change in this PR, the notion of the first and second symmetry in OrientationRegion.from_symmetry(s1, s2=C1) changes meaning, I think. I suggest to change parameter names to start, end (Python does not allow from), and remove the default C1. C1 should really be the default for start, but then end would also need a default, which isn't possible to define.
An alternative would be to allow one or two symmetries to be passed as *args and set start to C1 if only one symmetry was passed. This would effectively maintain the syntax of OrientationRegion.from_symmetry(D6) which is currently used some places in the code.
Another alternative is two flip the order end, start=C1 (keep the current parameter order but flip where they are used internally).
Talked to @anderscmathisen offline, reduction to the FZ now seems to make sense for his use.
Thanks @hakonanes . I hope I get some chance to test this soon on the dataset where I had the issues in the past. What is the exact version number of the new orix version which includes this change? I will probably install in a test environment initially and not destroy my old install just yet.
That would be great. This change only lives in my fork so far, which you should be able to install into an environment like so:
pip install git+https://github.com/hakonanes/orix.git@fix-misorientation-reduce
If reviewers agree that his change is necessary, I anticipate that the change will be part of the next minor release v0.12, which should be released shortly after this PR is merged.
@hakonanes I have installed in my testing environment, run it on the old data, and found that the symmetry reduction works more reliably on the Silicon misorientations dataset that was giving problems before.
Happy to hear that, thank you for doing the tests. Please report back if you still find inconsistencies.
FYI, @DorianDepriester checked out the changes in this PR on his calculation of weighted mean orientations in https://github.com/pyxem/orix/issues/434#issuecomment-1513203390 and got results in line with his expectations.
Sorry to spoil the party, but this new order does not work correctly in reduce for 622 symmetry.
Attached is the result of doing cluster analysis using symmetry reduction for HCP Ti using the old method.
And now here is the same workbook run with the new method. The results from the old method are quantitatively sensible. The new method is screwy. So, something about your order of rotations in symmetry reduction in hexagonal is wrong.
Thank you for sharing this test, @maclariz. I believe this is related to (inconsistent?) alignment of crystal axes with symmetry operations. I've encountered issues with point group -3m myself (chromium nitride in duplex steel) using the changes in this PR.
Given this issue, I suggest to wait with the change in the left/right symmetry of map_into_symmetry_reduced_zone() until we are sure of all alignments of crystal axes with symmetry operations. To easily debug these things, I suggest we implement automatic plotting of symmetry elements in the stereographic projection for all defined point groups. We can then convince ourselves of the alignments, before continuing with this change.
I'll make another PR for the changes in this PR not strictly related to the left/right symmetry change.
To easily debug these things, I suggest we implement automatic plotting of symmetry elements in the stereographic projection for all defined point groups.
I've started to work on this in https://github.com/hakonanes/orix/tree/plot-symmetry-operations. I anticipate it will take some time.
Check out this pull request on ![]()
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB