-
Notifications
You must be signed in to change notification settings - Fork 49
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
Minor maintenance updates before 0.12.0 release #478
Conversation
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
5ce1ade
to
63621ec
Compare
The only thing left I'd like to look at is the new gallery example on sampling reduced rotations by @CSSFrancis: orix/examples/rotations/sampling_rotations.py Lines 2 to 45 in d7c1b9a
A couple questions:
|
If it's OK with you @CSSFrancis, I'd like to re-work the example a bit along those lines. |
I think that is fine for the most part but it is sometimes nice to point out how someone can get that information from a
This is also related to the orientation mapping. I guess the reason is that we want the rotations because that is the result of the orientation mapping code. I think this is a good example of converting rotations to vectors for visualization by essentially defining a beam direction. You lose in-plane rotation information but in this case there isn't any. Maybe this is an anti-pattern and I don't understand the |
@hakonanes I don't have a great understanding of |
This is a good point. I've opened #480 to discuss this further.
This is what you added in May this then be a more descriptive example? import matplotlib.pyplot as plt
import numpy as np
from orix import plot, sampling
from orix.quaternion import symmetry
from orix.vector import Vector3d
# Input: sampling resolution and symmetry
res = 2 # Degrees
laue = symmetry.D4h # 4/mmm
R = sampling.get_sample_reduced_fundamental(res, point_group=laue)
# Show equivalence of vectors v1 and v2
v1 = R * Vector3d.zvector()
v2 = sampling.sample_S2(res)
v2 = v2[v2 <= laue.fundamental_sector]
assert np.allclose(v1.data, v2.data)
# True
fig, (ax0, ax1) = plt.subplots(
ncols=2, subplot_kw={"projection": "ipf", "symmetry": pg}, layout="tight"
)
ax0.scatter(v1, s=5)
ax1.scatter(v2, c="C1", s=5)
ax0.set_title("Rotated Z vectors", loc="left")
ax1.set_title("Directly sampled", loc="left")
fig.suptitle("Vectors in the fundamental sector of 4/mmm", y=0.8) |
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
…th zsh Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
f4e4003
to
929ea6d
Compare
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
929ea6d
to
661a73e
Compare
Perfect!
That is much more descriptive :) I don't think I thought as much as I should about what the example should show. |
I've added the above example to the examples gallery: https://orix--478.org.readthedocs.build/en/478/examples/rotations/rotations_mapping_fundamental_sector.html I've got the following questions regarding the new function
We need to change this function for it to align with the rest of orix. |
I've updated the TODO list in the top comment with what's left before we're ready for the 0.12.0 release. |
I'd imagine that this is a holdover from
This is most likely a bug. And using the fundamental sectors.vectors would be much better.
Honestly coming from a non crystallography background I looked in Edit: This doesn't make sense... I'm mixing this up a bit. Let me try again Edit x2: Okay now this makes more sense. |
Thank you for clarifying! Since the The below is one route to some nice high-symmetry zone axes is. If it's OK with you @CSSFrancis, I'll replace the function and corresponding tests with this example. from orix.crystal_map import Phase
from orix.vector import Miller
phase = Phase(point_group="mmm") # m-3m, 6/mmm, 4/mmm, -3m, mmm, 2/m
t = Miller.from_highest_indices(phase, uvw=[1, 1, 1])
t = t.in_fundamental_sector() # Project all to the fundamental sector
t = t.unit.unique(use_symmetry=True).round() # Unit vectors ensure e.g. [222] == [111]
print(t)
# Miller (7,), point group mmm, uvw
# [[0. 0. 1.]
# [0. 1. 1.]
# [0. 1. 0.]
# [1. 1. 1.]
# [1. 0. 1.]
# [1. 1. 0.]
# [1. 0. 0.]] Plotting the annotated vectors, only showing the fundamental sector fig = t.scatter(
vector_labels=[str(vi).replace(".", "") for vi in t.data],
text_kwargs={"size": 15, "offset": (0, 0.02), "bbox": {"fc": "w", "pad": 1, "alpha": 0.75}},
return_figure=True,
)
fig.axes[0].restrict_to_sector(t.phase.point_group.fundamental_sector) |
I think that is useful but it would be nice to include an example the returns the Rotations as well as the vectors for simulation purposes. I think a workflow like: import diffsims as ds
from orix.crystal_map import Phase
phase = Phase.from_cif("strucuture.cif")
sim = ds.simulations.Simulation()
zone_axes_patterns = sim.get_zone_axes_patterns(phase) is something we should be able to do fairly easily. If this is only used in I think something like the ASTAR/ JEMS GUI is something that we could fairly easily replicate with orix/diffsims and it would be very useful to a lot of people. |
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
…directions Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
I've added the above example with an additional section showing how to get the rotations rotating the Z-vector (0, 0, 1) onto the high-symmetry crystal directions. To have the syntax you suggest in diffsims, I suggest to make a function there instead of in orix. My feeling is that orix should be free of diffraction-specific functionality and stick to crystal orientations and directions. It may be that a more specific class method could be added to the |
879cc18
to
88576a5
Compare
@CSSFrancis, I'd appreciate it if you could have a look at the new examples, the rephrased docstring of |
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
…ser facing change Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
88576a5
to
5d00993
Compare
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.
@hakonanes This all looks very nice.
Unrelated to this PR, but it would be a good idea to all the Rotation.from_align_vectors
to take multiple vectors so you could do:
vz = Miller(uvw=[0, 0, 1], phase=t.phase)
R = Rotation.identity(t.size)
for i, t_i in enumerate(t):
R[i] = Rotation.from_align_vectors(t_i, vz)
print(R)
# Change to
R = Rotation.from_align_vectors(t, vz)
It's probably not super simple as the align vectors can handle the case where you are trying to align multiple vectors but we could also just add the function Rotation.from_align_vector
which assumes a direct rotation from one vector to another.
Signed-off-by: Håkon Wiik Ånes <hwaanes@xnovotech.com>
Good point. Well, in the API, we only support the case where the two end point vectors have the same size. So, we could just say that when the sizes differ, we do broadcasting, instead of raising an error. In the simplest case, we can support 1-to-n and n-to-1. It may be slow, since the SciPy function we call does not support broadcasting, but it is possible. |
LGTM! @hakonanes You can merge if you would like. |
Description of the change
This PR contains a few maintenance updates before the 0.12.0 release.
In particular, one test,
test_restrict_to_fundamental_sector()
, failed on my Mac with Python 3.11 and Matplotlib 3.8.3. The relevant part of the test ensures that the edges of the fundamental sectors of C1 and C6 are different, but failed because a different number of edges were returned. I'm not sure why. Anyways, to ensure the edges are different, we only need to check the first couple edge vectors. I've made it so we check the first ten.Other changes:
pytest-rerunfailures
on GitHub CI. Package added as a test dependency.Progress of the PR
get_sample_zone_axis()
get_sample_zone_axis()
For reviewers
__init__.py
.section in
CHANGELOG.rst
.__credits__
inorix/__init__.py
and in.zenodo.json
.