adding ancillary capabilities from PR442

[argerlt]

Description of the change

Implements some ancillary capabilities from #442 that were never merged into the develop branch.
I added @hakonanes as a co-author since I essentially copied his homework on this one. Contains two major changes:

1. Renames map_into_symmetry_reduced_zone() to reduce(), the value of which @hakonanes summed up nicely previously:

The other major change is naming: map_into_symmetry_reduced_zone() -> reduce(), inspired by the attractive naming of scipy.spatial.transforms.Rotation.reduce(). ...
... 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.

2. adds Rotation.from_path_ends, which is roughly equivalent to Vector3d.from_path_ends.

Still requires unit tests and examples.

After this is merged, I think the only things missing from #442 is the proper handling of misorientation symmetry, and the updated tutorials

Progress of the PR

• Docstrings for all functions
• Unit tests with pytest for all lines
• Clean code style by running black via pre-commit

Minimal example of the bug fix or new feature

WIP

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__ in orix/__init__.py and in
  .zenodo.json.

[argerlt]

@hakonanes , @CSSFrancis , @viljarjf, anyone got advice on how to unit test Orientation.from_path_ends? an easy out would be to just check that the size of the produced paths are the correct length, and the spacing is consistent between points, but that doesn't really test if the paths are in the correct spot. Toy example below to show off the feature in question:

from orix.quaternion import Orientation
from orix.quaternion.symmetry import O
import numpy as np
import matplotlib.pyplot as plt

rot_path = Orientation(
    data=np.array(
        [
            [1, 0, 0, 0],
            [1, 1, 0, 0],
            [1, 0, 1, 0],
            [1, 0, 0, 1],
            [1, 0, -1, 0],
            [1, 0, 0, -1],
            [1, 0, 0, -1],
        ]
    )
)
m3m_path = Orientation(
    data=np.array(
        [
            [1, 0, 0, 0],
            [2, 1, 0, 0],
            [3, 0, 1, 0],
            [4, 0, 0, 1],
            [5, 0, -1, 0],
            [6, 0, 0, -1],
            [7, 0, 0, -1],
            [8, 1, 0, 0],
            [9, 0, 1, 0],
            [10, 0, 0, 1],
            [11, 0, -1, 0],
            [12, 0, 0, -1],
            [13, 0, 0, -1],
        ]
    ),
    symmetry=O,
)

fig, ax = plt.subplots(1, 2)
# plot in homochoric space, no symmetry
Orientation.from_path_ends(rot_path.reduce(), True).reduce().scatter(
    figure=fig, position = [1,2,1], marker=">", c=np.arange(700)
)

# plot in rodrigues space, cubic
Orientation.from_path_ends(m3m_path.reduce(), True).reduce().scatter(
    figure=fig, position = [1,2,2], marker=">", c=np.arange(1300)
)

[hakonanes]

Good stuff. However, many things need changing!

[hakonanes]

I don't have a good suggestion for testing orientation paths.

However, here's another test which may be useful, just checking that a path from identity to some other one produces the same as a fiber defined by one axes and increasing angles:

    def test_from_path_ends_fiber(self):
        Q1 = Quaternion.identity()
        Q2 = Quaternion.from_axes_angles([1, 1, 1], 60, degrees=True)
        Q12 = Quaternion.stack((Q1, Q2))
        Q_path1 = Quaternion.from_axes_angles([1, 1, 1], np.arange(59), degrees=True)
        Q_path2 = Quaternion.from_path_ends(Q12, steps=Q_path1.size)
        assert np.allclose(Q_path1.dot(Q_path2), 1, atol=1e-3)

[hakonanes]

Almost there. I suggest to add examples to the examples gallery showing how to use this, one for each of the three classes.

[hakonanes]

I don't think this is a good pattern. If the points aren't misorientations, the quaternion method should be used instead. We should do either of

1. allow a symmetry parameter
2. assume points are misorientations

[hakonanes]

I prefer assuming points are misorientations.

[argerlt]

Isn't that what this already does? regardless of what you pass in, it treats it like a quaternion, but then it casts it to a misorientation (I did it this way because it works with Orientation as well). Then, if there is symmetry, it copies it over, and if not, it doesn't.

This is what I'm checking with test_orientation.test_from_path_ends, it matches up with what i would expect, where the class used to call from_path_ends determines the class of the produced object, and if symmetry exists, it's copied over as well.

    q = Quaternion.random(10)
    r = Rotation.random(10)
    o = Orientation.random(10, Oh)
    m = Misorientation.random(10, [D3, Oh])

    # make sure the result is the correct class.
    a = Quaternion.from_path_ends(q)
    b = Rotation.from_path_ends(q)
    c = Orientation.from_path_ends(r)
    d = Quaternion.from_path_ends(o)
    e = Orientation.from_path_ends(q)
    f = Misorientation.from_path_ends(m)
    g = Orientation.from_path_ends(o)
    assert isinstance(a, Quaternion)
    assert isinstance(b, Rotation)
    assert isinstance(c, Orientation)
    assert isinstance(d, Quaternion)
    assert isinstance(e, Orientation)
    assert isinstance(f, Misorientation)
    assert isinstance(g, Orientation)

    # make sure symmetry information is preserved.
    assert f.symmetry == m.symmetry
    assert g.symmetry == o.symmetry

[hakonanes]

My point is that we should accept a symmetry parameter because this doesn't make much sense to me:

import orix.quaternion as oqu

Q = oqu.Quaternion.random(2)
M = oqu.Misorientation.from_path_ends(Q)

If the user just want a path trough rotation space, they should use Quaternion.from_path_ends(). If they wanted misorientations through misorientation space, they should pass symmetries along with the input quaternions.

[argerlt]

EDIT: Nevermind, this point is valid but seperate from the issue at hand.

@hakonanes, Coming back to this months later:

It occurs to me that it's incorrect/misrepresentative to claim this function traces a path through "Misorientation Space", as from_path_ends does not choose a path with respect to symmetry. This is a bit like claiming Miller space and Vector space are the same.

Additionally, I don't you should (or maybe even can, for that matter) make this function symmetry-aware, because you end up in a traveling salesman conundrum. Finding the shortest path from A-to-B or B-to-C is fine, but A-to-B-to-C-to-D... etc and checking all the possible symmetry combinations gets hard fast, and is way outside the scope of this function.

With that in mind, I think maybe this should only be a function in Quaternion, and if it's called from an inherited class like Misorientation or Orientatation, it returns a Quaternion anyway, to hammer home this point. I would then add a note to the effect of

This function traces a path between points in SO(3)|C_1, in which there is one and only one direct path between every point. The equivalent is not well-defined for (mis)orientations, which have multiple symmetrically-equivalent points with nonequivalent paths between them, thus creating ambiguity in the definition of "shortest path" or "direct path" for symmetry-aware spaces. Additionally, the concept of "paths" does not hold with regard to inversions either.  
Thus, to make this point clear, the function always returns a path of Quaternions, which can then be converted as desired by the user to a symmetry-aware object of their choice.

Thoughts?

[argerlt]

I don't have a good suggestion for testing orientation paths.

However, here's another test which may be useful, just checking that a path from identity to some other one produces the same as a fiber defined by one axes and increasing angles:

    def test_from_path_ends_fiber(self):
        Q1 = Quaternion.identity()
        Q2 = Quaternion.from_axes_angles([1, 1, 1], 60, degrees=True)
        Q12 = Quaternion.stack((Q1, Q2))
        Q_path1 = Quaternion.from_axes_angles([1, 1, 1], np.arange(59), degrees=True)
        Q_path2 = Quaternion.from_path_ends(Q12, steps=Q_path1.size)
        assert np.allclose(Q_path1.dot(Q_path2), 1, atol=1e-3)

This seems good enough. Also, as stated in the docstring, paths are not symmetry-aware, so this plus the expected size checks, plus the proper symmetry copying seems sufficient.

[hakonanes]

I suggest to not mark a PR as a draft after there's been a review.

[argerlt]

I suggest to not mark a PR as a draft after there's been a review.

I was trying to signify I had started fixing this PR, but it wasn't yet to the point where it was worth maintainers taking time to review it. If there is a better way to signify as much, I can switch.

I have to swap between computers throughout the week based on other work happening, so pushing partial patches is convenient, and also a way to get unofficial input from others.. @CSSFrancis and I were using labels for a bit, but I was trying out the Draft designation instead since only maintainers can add labels, and I thought draft's might be more inclusive.

[hakonanes]

OK, I see. I'd say after the first review, it's not a draft anymore. But that doesn't stop you from updating the branch further!

[hakonanes]

What I mean above (https://github.com/pyxem/orix/pull/558/files#r2215490352) is that we should drop the check and just do this:

Suggested change

	if hasattr(points, "_symmetry"):
	path._symmetry = points._symmetry
	path._symmetry = points._symmetry

If not (mis)orientations are given, this will raise an attribute error. Users should see this error to learn that they should instead use the equivalent rotation or quaternion method, instead of the call passing silently.

If we want to be more kind to the user, and perhaps save unnecessary computation, we can make a check at the top for type:

if not isinstance(points, Misorientation):
    raise TypeError(f"Points must be a (mis)orientation, not of type {type(points)}")

After this is 100% safe to access the symmetry.

[hakonanes]

I can make this change, if you'd like.

[argerlt]

Okay, I think this is fine.

I added your suggestion, but then when going through unit tests, I realized the solution given as-is could recast misorientations as orientations, (sort of bad) but also rewrite the right-only symmetry tuple with a left-right symmetry tuple (VERY bad). I believe adding Orientation.from_path_ends fixes this.

If you don't like this version, feel free to change it. As long as I can draw lines through orientation space, I'm happy.

[argerlt]

@hakonanes , give me 24 hours on this one please. I believe all the code is good to go, but the example I wrote is half-baked and needs some work.

[hakonanes]

No problem!

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[argerlt]

I finished the example (see picture below), cleaned up some code, removed some obsolete refrenes to map_into_symmetry_reduced_zone, and also ended up adding a HomochoricPlot class since it made sense.

This PR pre-assumes #583 is approved, as it fails without that update.

[argerlt]

This PR was broken up into #558 and #600, and is now redundant.