Refactor Electron Diffraction Simulations

[CSSFrancis]

Description of the change

Based on some changes from @din14970 I figured I would pick up this work a little bit

Progress of the PR

• Add ConstrainedRotation class for building rotations for accelerated template matching
• Add plotting method for ConstrainedRotation
• Add ability to use ASE or diffpy for
• Add nicer representations for StructureLibrary
• Docstrings for all functions
• Unit tests with pytest for all lines
• Clean code style by running black

Minimal example of the bug fix or new feature

from diffsims.generators.rotation_list_generators import get_reduced_fundamental_zone_grid
from orix.quaternion import symmetry
import matplotlib.pyplot as plt

syms = [symmetry.C1, symmetry.C2, symmetry.C4, symmetry.C6, symmetry.Oh]
sym_names = ["C1", "C2", "C4", "C6", "Oh"]

rots = [get_reduced_fundamental_zone_grid(2, point_group=s) for s in syms]
fig, axs = plt.subplots(1, len(syms), figsize=(15, 3))
for r, ax, s in zip(rots, axs, sym_names):
    r.plot(ax=ax, s=2)
    ax.set_title(s)
plt.show()

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 diffsims/release_info.py and
  in .zenodo.json.

[CSSFrancis]

@hakonanes Okay so this is starting to look better now but there might be a bit to go and this is going to probably end up being way too large of a PR. This rewrites a lot of the simulation stuff in diffsims and makes a lot of api changes. Should we try to deprecate things or just try and go for it and rip the bandaid off with a new version of diffsims which should have a much more stable API?

I can try split up some of this as well if that makes more sense.

A couple more things:

I think that the Structure Library Class is redundant. I'm already of the opinion that there are too many classes, but I think it is reasonable to just directly create a DiffractionGenerator object like:

from diffsims.crystallography import CrystalPhase
from diffpy.structure import Lattice, Atom, Structure
from diffsims.generators.diffraction_generator import DiffractionGenerator
from diffsims.generators.library_generator import DiffractionLibraryGenerator

# Creating a crystal phase to which we will make the template library from
crystal = CrystalPhase("al",
                  space_group=225,
                  structure=Structure(lattice=Lattice(4.04, 4.04, 4.04, 90, 90, 90),
                  atoms=[Atom("Al", [0, 0, 1])],),
                  )

# Creating a DiffractionGenerator to generate the diffraction patterns
calc = DiffractionGenerator(300.0)  # 300 keV
# Creating a DiffractionLibraryGenerator to generate the library
dfl = DiffractionLibraryGenerator(names=["al_phase"],
                                                          phases=[crystal,],
                                                          orientations = [ crystal.constrained_rotation(),],
                                                          electron_diffraction_calculator=calc,
                                                          reciprocal_radius=1.0)

[hakonanes]

I'm sorry, but I'm not familiar enough with the spot pattern diffraction simulation setup to review this PR as thoroughly as I would like to.

However, I can give my two cents regarding the snippet you propose above:

• The initialization of CrystalPhase is identical to the initialization of orix' Phase. I find it difficult to see the need for this class. I also find the name too similar.
• The diffraction library generator can just take the names from Phase.name, right?
• We have an orix.sampling module for sampling 3D vectors and orientations. Is it possible to add the constrained_rotation() sampling method as a function to that module, taking a point group symmetry as input? If we do this, is there then a need for CrystalPhase? This approach seems more natural to me. Then this particular sampling method can be used in other circumstances as well.

Regarding the setup of simulations of spot patterns, I've made a comment in the relevant issue #198 (comment).

If possible, I suggest to support two routes for creating simulation libraries in one or two minor releases. Users using the old route should be warned somehow that they should use the new route. I know that maintenance resources are extremely scarce. And that this requires more work. But, it would be very welcome, since my impression is that template matching in pyxem is more and more used.

I'm sorry I can't be of more help with the implementation.

[CSSFrancis]

I'm sorry, but I'm not familiar enough with the spot pattern diffraction simulation setup to review this PR as thoroughly as I would like to.

Totally fine. I might be able to ping a couple of people to review the actual spot pattern diffraction simulation. The advice on how to structure it it very valuable.

However, I can give my two cents regarding the snippet you propose above:

• The initialization of CrystalPhase is identical to the initialization of orix' Phase. I find it difficult to see the need for this class. I also find the name too similar.

I'm okay with that! Less classes are a good thing in this case as I already feel there are too many!

• The diffraction library generator can just take the names from Phase.name, right?

Hmm, yea it should. We just probably have to handle the case of two phases with the same name. But we can just add "name_1" and "name_2" in that case.

• We have an orix.sampling module for sampling 3D vectors and orientations. Is it possible to add the constrained_rotation() sampling method as a function to that module, taking a point group symmetry as input? If we do this, is there then a need for CrystalPhase? This approach seems more natural to me. Then this particular sampling method can be used in other circumstances as well.

Yea let's get rid of the CrystalPhase class. :) I've been fighting to get it to work and it really doesn't. I liked the idea of having some Phase object and then automatically determining things like the ZAP and the constrained_rotations from the phase. How do you feel about adding things like get_constrained_roattion, get_zone_axis_rotations and get_sample_fundamental to the Phase class?

rots = phase. get_constrained_rotation(1.0)
# vs
from orix.sampling import  get_constrained_rotation
get_constrained_rotation(1.0, phase.space_group, phase.point_group)

Just a thought...

Regarding the setup of simulations of spot patterns, I've made a comment in the relevant issue #198 (comment).

Thanks! I think we want the workflows between kikuchipy/diffsims to be as parallel as possible so your input is really helpful there. I can handle the diffraction spot simulations stuff. I'm not sure if you ever used JEMS, but I would eventually like something like the ability use the stereographic projection/ kikuchi patterns to navigate the Zone axis patterns for some crystal.

If possible, I suggest to support two routes for creating simulation libraries in one or two minor releases. Users using the old route should be warned somehow that they should use the new route. I know that maintenance resources are extremely scarce. And that this requires more work. But, it would be very welcome, since my impression is that template matching in pyxem is more and more used.

I'm happy to accommodate. I'm probably going to add the deprecation wrappers here as well and then I can start making some smaller PRs

I'm sorry I can't be of more help with the implementation.

This is super helpful :)

[hakonanes]

Hmm, yea it should. We just probably have to handle the case of two phases with the same name. But we can just add "name_1" and "name_2" in that case.

Yes, that is a reasonable solution. The user of course shouldn't have a phases with identical names in the list, but raising an error might be too harsh. The phase name can just be changed after indexing anyway.

How do you feel about adding things like get_constrained_roattion, get_zone_axis_rotations and get_sample_fundamental to the Phase class?

These functions return rotations or orientations. When we want orientations, the only inputs are sample resolution and proper point group symmetry. We should therefore add them as class methods to the rotation and orientation classes. (Specifying a Phase just for this should be unnecessary.)

However, I'm not sure returning rotations with the first Euler angle always 0 and rotations around a "zone axis" to orix. Perhaps the former is general enough. The latter is diffraction specific and should thus reside in diffsims. But, having a separate utility function for get_zone_axis_rotations() seems inelegant... I'm not sure what the best solution is, to be honest.

[CSSFrancis]

Hmm, yea it should. We just probably have to handle the case of two phases with the same name. But we can just add "name_1" and "name_2" in that case.

Yes, that is a reasonable solution. The user of course shouldn't have a phases with identical names in the list, but raising an error might be too harsh. The phase name can just be changed after indexing anyway.

How do you feel about adding things like get_constrained_roattion, get_zone_axis_rotations and get_sample_fundamental to the Phase class?

These functions return rotations or orientations. When we want orientations, the only inputs are sample resolution and proper point group symmetry. We should therefore add them as class methods to the rotation and orientation classes. (Specifying a Phase just for this should be unnecessary.)

Just because I'm not very used to simulations where everything isn't in a straight line, the distinction here is that a orientation just refers to a vector direction within some object and a rotation can be defined by two vectors.

However, I'm not sure returning rotations with the first Euler angle always 0 and rotations around a "zone axis" to orix. Perhaps the former is general enough. The latter is diffraction specific and should thus reside in diffsims. But, having a separate utility function for get_zone_axis_rotations() seems inelegant... I'm not sure what the best solution is, to be honest.

I would like everything in diffsims to work for any geometry. What about something like get_zone_axis_rotations which includes an axis_of_rotation = (0,0,1) for the z direction by default and then return a Rotation object?

[CSSFrancis]

@hakonanes I'm still trying to figure a couple of things out. Maybe I should be using the Vector3D class to define the orientations for the Library.

One option would be to have both a beam_direction: Vector3d = Vector3d[(0, 0, 1)] and a rotation_direction with the calculate_ed_data function and then create the Rotation matrix using the from_align_vectors function. That is closer to what is currently being done :). That would help with the plotting of the orientations as well.

I'm a little unsure of how to visualize the stereographic projections from a Rotation object.

[CSSFrancis]

Close in favor of #201