Detector orientation with euler angles: simulations from master patterns and GeometricalKikuchiPatternSimulation #691
Conversation
|
Hi @tgwoodcock, Thank you for this PR and for the kind words! Glad you use kikuchipy and find it useful :) I don't have time to look at your changes in detail now. From the top comment it seems like they fix a bug and add some nice new features. Although I believe we have to iterate on the additions to the public API (naming of the Euler angles and matrices). I hope to get back to you within a week at the latest so we can get this in. |
tgwoodcock
force-pushed
the
detector_orientation
branch
from
a2a81fe
to
b0d7725
Compare
|
Hi @tgwoodcock, is it OK if I push changes that makes the test suite pass? And have you done any benchmarking of refinement and projection from master pattern to detector after making your changes? |
| @@ -239,10 +240,96 @@ def __repr__(self) -> str: | |||
| f"sample_tilt={sample_tilt}, " | |||
| f"tilt={tilt}, " | |||
| f"azimuthal={azimuthal}, " | |||
| f"euler={tuple(self.euler)}, " | |||
There was a problem hiding this comment.
I suggest to drop this, as the string representation is already too long. The same information is anyways contained in the tilt and azimuthal angles.
There was a problem hiding this comment.
If we define a third angle (see other comments), then I guess we could drop euler and just have the other 3 angles instead.
| self.sample_tilt = float(sample_tilt) | ||
| self._euler = np.array([self.azimuthal, 90.0 + self.tilt, 0.0]).astype(float) |
There was a problem hiding this comment.
Seems to me like we get the same functionality if we just make EBSDDetector.euler a dependent property
@property
def euler(self) -> np.ndarray:
return np.array([self.azimuthal, 90. + self.tilt, 0.], dtype=float)This way we don't have to handle the tilts separately. We should have only one way of updating the angles, and I believe this should be via EBSDDetector.tilt and EBSDDetector.azimuthal.
What do you think?
There was a problem hiding this comment.
There are plenty of examples where the third detector Euler angle is non-zero. At least for the Oxford Instruments software, the detector orientation is stored in the h5oina as Euler angles. I think there is a case for being able to set the detector Euler angles directly e.g. when using the kp.load() function to read a dataset. On the other hand, if you prefer to define the angles separately, it would be possible to define a third angle, something like EBSDDetector.twist, which would be equivalent to EBSDDectector.euler[2].
| @@ -495,6 +582,30 @@ def r_max(self) -> np.ndarray: | |||
| corners[..., 3] = self.x_min**2 + self.y_min**2 # Lo. left | |||
| return np.atleast_2d(np.sqrt(np.max(corners, axis=-1))) | |||
|
|
|||
| @property | |||
| def u_s(self) -> np.ndarray: | |||
There was a problem hiding this comment.
It is a great idea to have this transformation as a dependent property.
We aim for as tight an integration as possible with orix, so this should return a orix.quaternion.Rotation. People can then get the matrix themselves by calling Rotation.to_matrix().
We must call the property something self-explanatory. I suggest EBSDDetector.sample_to_detector. Do you have another suggestion? We can give the reference to the relevant equation in the Britton et al. (2016) paper, and even give the U_s symbol in the docstring.
There was a problem hiding this comment.
Sounds good, it would be easier to have a more self-explanatory name.
| return u_s | ||
|
|
||
| @property | ||
| def u_s_inv(self) -> np.ndarray: |
There was a problem hiding this comment.
I suggest to drop this and instead explain that the rotation can be obtained from ~EBSDDetector.sample_to_detector.
| @@ -518,6 +629,7 @@ def load(cls, fname: Path | str) -> EBSDDetector: | |||
| "binning", | |||
| "tilt", | |||
| "azimuthal", | |||
| "euler", | |||
There was a problem hiding this comment.
Since this is a dependent property, we don't have to include it in IO.
There was a problem hiding this comment.
If we go down the route of defining a third angle (e.g. twist), then that could be added to IO and we can safely drop euler.
| tilt: float, | ||
| azimuthal: float, | ||
| sample_tilt: float, | ||
| u_s_inv: np.ndarray, |
There was a problem hiding this comment.
Again, we should rename to something self-explanatory, such as om_detector_to_sample (om is used for orientation matrix in orix and elsewhere).
| r_g_array[i, 1] = (det_gn_y[rows[i]] - y_half_step) * pcz | ||
| r_g_array[i, 2] = pcz | ||
|
|
||
| # rotate vectors from detector reference frame CSd to |
There was a problem hiding this comment.
I'd remove the comment and rather rename the orientation matrix variable to be self-explanatory.
|
|
||
| phase = kp.data.nickel_ebsd_master_pattern_small().phase | ||
|
|
||
| pqr_p = [(1, 1, 1), (2, 0, 0), (2, 2, 0), (3, 1, 1)] |
There was a problem hiding this comment.
I'd rename to hkl which is self-explanatory.
| u_a = phase.structure.lattice.base | ||
|
|
||
| def setup_detector_sim_and_u_os(self, tilt, azimuthal, euler_2): | ||
| """Setup the detector with the correct tilt, azimuthal and |
There was a problem hiding this comment.
I'd consider this docstring to be superfluous as the code is easy to read and self-documenting. I'd remove it.
| return None | ||
|
|
||
|
|
||
| def get_coordinate_conversions(detector): |
There was a problem hiding this comment.
It looks like the next two functions may be useful to have as functionality accessible via the EBSD detector?
There was a problem hiding this comment.
Yes, they could easily be moved into the EBSDDetector class.
Added an attribute self._euler, which holds the three detector Euler angles in degrees. These describe the orientation of the detector. The attributes self.tilt and self.azimuthal were made into "non-public" attributes i.e. self._tilt and self._azimuthal. self._tilt, self._azimuthal and self._euler are set directly in self.__init__() i.e. NOT by calling the setters. These values will therefore always be self consistent: self._euler is not an argument for self.__init_(). Getters and setters were added for tilt, azimuthal and euler. These are designed to retain self.consistency - e.g. if you set self.azimuthal or self.tilt, self.euler will be updated accordingly and vice versa. self.euler[0] == self.azimuthal self.euler[1] == 90 + self.tilt The methods EBSDDetector.save() and EBSDDetector.load() were updated so that the EBSDDetector.euler property is additionally saved. This is needed because EBSDDetector.euler[2] may not be zero and is currently not represented by any other EBSDDetector property or attribute. The test method TestSaveLoadDetector.test_save_load_detector() in tests/test_detectors/test_ebsd_detector.py has been updated to include an extra assert statement to check that the saved and read euler angles are the same. All tests in TestSaveLoadDetector pass with the current code. The tests test_set_tilt(), test_set_azimuthal() and test_set_euler() were added to the class TestEBSDDetector in tests/test_detectors/test_ebsd_detector.py. These help to ensure that when setting one of the properties, the others are updated for consistency. All tests in tests/test_detectors/test_ebsd_detector.py pass with the current code. Signed-off-by: tgwoodcock <tgwoodcock@users.noreply.github.com>
These properties return the orientation matrix u_s, which transforms vectors in the sample coordinate system CSs, to the detector coordinate system, CSd, and the inverse matrix, u_s_inv,for the opposite transformation. Signed-off-by: tgwoodcock <tgwoodcock@users.noreply.github.com>
…ed test The __repr__() method in the EBSDDetector class in src/kikuchipy/detectors/ebsd_detector.py was modified so that the detector euler angles are also printed. The associated test, TestEBSDDetector.test_repr() in tests/test_detectors/test_ebsd_detector.py was modified accordingly so that it still passes. Signed-off-by: tgwoodcock <tgwoodcock@users.noreply.github.com>
Modified the calculation of the orientation matrix u_s_bruker in KikuchiPatternSimulator.on_detector(). Previously u_s_bruker was determined from the 'total_tilt', which only considered EBSDDetector.sample_tilt and EBSDDetector.tilt but NOT EBSDDetector.azimuthal. Now EBSDDetector has the detector Euler angles, which describe the full orientation of the detector and these can be used to calculate u_s_bruker. In this way, the full orientation of the detector can be considered when making simulations to overlay on patterns. This will give a much better fit for cases where the detector azimuthal angle was non-zero (i.e. EBSDDetector.euler[0] != 0) or where the third detector Euler angle was non-zero. Signed-off-by: tgwoodcock <tgwoodcock@users.noreply.github.com>
….util._master_pattern.py The functions _get_direction_cosines_for_fixed_pc() and _get_direction_cosines_for_varying_pc() have been modified. The previous functions used an algorithm adapted from EMSoft to calculate the direction cosines of the detector pixels. This has now been changed and the functions use an orientation matrix approach, adapted from that in the tutorial paper by Britton et al. This enables the detector tilt, azimuthal and euler[2], i.e. all 3 detector euler angles, to be correctly incorporated in the pattern simulation. The function _get_direction_cosines_from_detector() has been updated to reflect the new signatures of the other two functions. Signed-off-by: tgwoodcock <tgwoodcock@users.noreply.github.com>
…sociated test The funtion _get_cosine_sine_of_alpha_and_azimuthal() has been removed from src/kikuchipy/signals/util/_master_pattern.py as it is no longer needed. The function import and associated test in tests/test_signals/test_ebsd_master_pattern.py has also been removed. Signed-off-by: tgwoodcock <tgwoodcock@users.noreply.github.com>
…direction_cosines_for_varying_pc() In tests/test_rotation/test_rotation.py: In TestRotationVectorTools.test_rotate_vector(), added the instantiation of an EBSDDetector so that the gnomonic_bounds can be passed into the new signature of _get_direction_cosines_for_fixed_pc(). Modified the arguments in the function call accordingly. In tests/test_signals/test_ebsd_master_pattern.py: In TestProjectFromLambert.test_get_direction_cosines(), updated the arguments in the function call to _get_direction_cosines_for_fixed_pc(). Added a further call to that function but WITHOUT .py_func and added a further assert statement to check that the results with and without .py_func are the same (as recommended in the contributing guidelines). In TestProjectFromLambert.test_get_direction_cosines_for_multiple_pcs(), updated the arguments in the function call to _get_direction_cosines_for_varying_pc(). Added a further call to that function but WITHOUT .py_func and added a further assert statement to check that the results with and without .py_func are the same (as recommended in the contributing guidelines). In TestProjectFromLambert.test_detector_azimuthal() As the function_get_direction_cosines_for_fixed_pc() has been modified, the small rotation in the EBSD pattern simulated from the master pattern for tilt!=0 and azimuthal!=0 is no longer present. The mean values of the data in the simulated patterns sim2 and sim3 in the above test are therefore very slightly different to the old values. The mean values have been updated. The above tests are now all passing. Signed-off-by: tgwoodcock <tgwoodcock@users.noreply.github.com>
…for indexing In src/kikuchipy/indexing/_refinement/_objective_functions.py, _refine_pc_objective_function() and _refine_orientation_pc_objective_function() were modified. Instead of calling _get_direction_cosines_for_fixed_pc() directly, an EBSDDetector is instantiated and _get_direction_cosines_from_detector() is called instead. This handles the changed function signature of _get_direction_cosines_for_fixed_pc(). In src/kikuchipy/indexing/_refinement/_solvers.py, _refine_orientation_solver_scipy() and _refine_orientation_solver_nlopt() were modified. The block under "if direction_cosines is None:" has been changed so that an EBSDDetector is instantiated and then _get_direction_cosines_from_detector() is called. Imports to these two modules have been added/modified accordingly. Signed-off-by: tgwoodcock <tgwoodcock@users.noreply.github.com>
The above function in the suite TestEBSDRefineOrientationPC has been modified by adding a trust region as an argument in the call to s.refine_orientation_projection_centre() with the method "differential_evolution". This was necessary because the new version of _refine_orientation_pc_objective_function() in indexing\_refinement\_objective_functions.py requires the instatiation of an EBSDDetector and thus the subsequent calculation of its gnomonic_bounds. If pc is (0, 0, 0), this leads to strange results which cause errors in the function _get_direction_cosines_for_fixed_pc() in signals\util\_master_pattern.py. Adding the trust region (same parameters as in the earlier call to s.refine_orientation_projection_centre() within the same test function) solves this problem. The test test_refine_orientation_pc_pseudo_symmetry_scipy() now passes, as do ALL the tests in tests/test_indexing/test_ebsd_refinement.py. Signed-off-by: tgwoodcock <tgwoodcock@users.noreply.github.com>
…GeometricalKikuchiPatternSimulation The class TestFitPatternDetectorOrientation has been added to tests/test_signals/test_ebsd_master_pattern.py. The class contains the test test_fit_detector_orientation(), which is parametrised with 8 sets of detector tilt, azimuthal and euler_2 angles, and lists of associated zone axes, which are on the pattern simulated under those conditions. For each set of parameters, we expect that the fit between the simulated pattern and the GeometricalKikuchiPatternSimulation is perfect. These tests are currently all passing. Signed-off-by: tgwoodcock <tgwoodcock@users.noreply.github.com>
tgwoodcock
force-pushed
the
detector_orientation
branch
from
b0d7725
to
22842a0
Compare
I hope I have fixed the problems now - the test suite passes on my Windows machine with Python3.12. Have updated my remote branch.
Not with refinement but I did benchmark getting the direction cosines. The performance was slower with the new version. I'll look into whether this can be improved. |
Description of the change
Hi @hakonanes and the kikuchipy team,
firstly, many thanks for kikuchipy - it's a great library which we use a lot in our work!
The focus of this PR is to include the detector Euler angles as a means of describing the full orientation of the detector and to use these in simulating EBSD patterns from master patterns and in producing lines to overlay on those patterns using
KikuchiPatternSimulator.on_detector().To achieve this, the following main changes have been made:
EBSDDetector, which is used to describe the full orientation of the detector via the Euler angles (in degrees).EBSDDetector.euler[0]is the same asEBSDDetector.azimuthalandEBSDDetector.euler[1]is the same asEBSDDetector.tilt + 90, getters and setters forEBSDDetector.tilt,EBSDDetector.azimuthalandEBSDDetector.eulerhave been added, which ensure consistency with the other properties when setting one of them.EBSDDetector.__repr__().EBSDDetector, which are orientation matrices used to transform from CSs to CSd and the reverse. The matrix "u_s_inv" is used in the new functions for getting the direction cosines of the detector (see 3.).KikuchiPatternSimulator.on_detector()so that the euler angles of the detector are now used in calculating the orientation matrix "u_s_bruker". Previously, only theEBSDDetector.tiltseemed to be considered.EBSDDetectorand orientation matrices, which consider all three detector euler angles (previously only tilt and azimuthal were considered)._get_cosine_sine_of_alpha_and_azimuthal(),TestFitPatternDetectorOrientationto tests/test_signals/test_ebsd_master_pattern.py. This checks the fit between an EBSD pattern simulated from a master pattern and the associatedGeometricalKikuchiPatternSimulation. This seemed to be worth testing because with the current version of the functions to get the direction cosines from the detector, there was a rotation of the EBSD simulation from the master pattern compared to theGeometricalKikuchiPatternSimulationif both tilt and azimuthal were non-zero:Whereas with the new method for getting the direction cosines in this PR, the EBSD pattern and
GeometricalKikuchiPatternSimulationfit perfectly for any combination of detector euler angles:The entire kikuchipy test suite passes. I was not able to build the documentation on Windows but I think everything should be fine with the tutorial notebooks.
Progress of the #PR
Minimal example of the bug fix or new feature
For reviewers
__init__.py.section in
CHANGELOG.rst.kikuchipy/__init__.pyand.zenodo.json.