Rotation of ReciprocalLatticeVector throws an error

[hakonanes]

Describe the bug
Rotation of a ReciprocalLatticeVector, g, with a Rotation from orix, R, throws an error. The multiplication uses Quaternion.__mul__(). There, the other being multiplied, here g, fails the test isinstance(other, Miller) and thus phase information is not passed on when creating the rotated ReciprocalLatticeVector in its __init__().

Code in orix: https://github.com/pyxem/orix/blob/a1be2697dc0cf02974b00d06d94272de77efeafa/orix/quaternion/quaternion.py#L238-L243

Minimal example

from orix.crystal_map import Phase
from orix.quaternion import Rotation
from diffsims.crystallography import ReciprocalLatticeVector


phase = Phase(point_group="m-3m")
g = ReciprocalLatticeVector(phase, [1, 1, 1])

R = Rotation.random()
g1 = R * g

throws

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
File /Users/hakon/kode/scratches/ds_scratch.py:5
      2 R = Rotation.random()
      3 om = R.to_matrix()
----> 5 g1 = R * g
      6 #g2 = g.rotate_from_matrix(om)

File /opt/homebrew/Caskroom/miniforge/base/envs/ds/lib/python3.11/site-packages/orix/quaternion/rotation.py:123, in Rotation.__mul__(self, other)
    121     return Q
    122 if isinstance(other, Vector3d):
--> 123     v = Quaternion(self) * other
    124     improper = (self.improper * np.ones(other.shape)).astype(bool)
    125     v[improper] = -v[improper]

File /opt/homebrew/Caskroom/miniforge/base/envs/ds/lib/python3.11/site-packages/orix/quaternion/quaternion.py:243, in Quaternion.__mul__(self, other)
    241         return m
    242     else:
--> 243         return other.__class__(v)
    244 return NotImplemented

File ~/kode/personal/diffsims/diffsims/crystallography/reciprocal_lattice_vector.py:105, in ReciprocalLatticeVector.__init__(self, phase, xyz, hkl, hkil)
    103 def __init__(self, phase, xyz=None, hkl=None, hkil=None):
    104     self.phase = phase
--> 105     self._raise_if_no_point_group()
    107     if np.sum([i is not None for i in [xyz, hkl, hkil]]) != 1:
    108         raise ValueError(\"Exactly one of `xyz`, `hkl`, or `hkil` must be passed\")

File ~/kode/personal/diffsims/diffsims/crystallography/reciprocal_lattice_vector.py:1265, in ReciprocalLatticeVector._raise_if_no_point_group(self)
   1259 def _raise_if_no_point_group(self):
   1260     \"\"\"Raise ValueError if the phase attribute has no point group
   1261     set.
   1262 
   1263     \"\"\"
-> 1265     if self.phase.point_group is None:
   1266         raise ValueError(f\"The phase {self.phase} must have a point group set\")

AttributeError: 'numpy.ndarray' object has no attribute 'point_group'"

The rotated ReciprocalLatticeVector with the phase information intact should be returned.

I'm not 100% sure how to fix this. ReciprocalLatticeVector should not subclass Miller, as the coordinates of the former can only be (hkl). Any input would be appreciated.

[viljarjf]

Phase information kan be kept by changing https://github.com/pyxem/orix/blob/develop/orix/quaternion/quaternion.py#L238 to if hasattr(other, "phase"):, which feels a little hacky.
If we override __neg__ in ReciprocalLatticeVector too, this issue is solved.

[CSSFrancis]

Should this even be possible?

1. The hkl value is not preserved
2. How do you handle multiple orations. Should that return multiple RLV (that is actually not ideal for the simulation purposes as the size of that array becomes large!)

[viljarjf]

I might be misunderstanding what rotating a hkl vector entails. What do you mean by preserving the hkl value? When rotating, should they not change?
If the shapes are compatible, a Rotation object with multiple rotations will distribute as expected, e.g. a RLV with shape (10,) and a Rotation with shape (4, 1) would create a RLV with shape (4, 10). This is how Miller works currently.

[CSSFrancis]

@viljarjf The HKL is dependent on the RLV and the lattice. If we have a set of RLV and we rotate them (and don't rotate the lattice in some way) calculating the hkl no longer works as expected and gives non physical values.

[viljarjf]

This is exactly the same operation as your rotate_from_matrix, apart from the default of Rotation being lab2crystal when rotate_from_matrix implicitly uses crystal2lab. Is that not what you would want?

[CSSFrancis]

import diffpy
import numpy as np
from diffsims.crystallography import ReciprocalLatticeVector
from orix.crystal_map import Phase
from orix.quaternion import Rotation

# Shared parameters
latt = diffpy.structure.lattice.Lattice(2.464, 2.464, 6.711, 90, 90, 120)
atoms = [diffpy.structure.atom.Atom(atype='C', xyz=[0.0, 0.0, 0.25], lattice=latt),
         diffpy.structure.atom.Atom(atype='C', xyz=[0.0, 0.0, 0.75], lattice=latt),
         diffpy.structure.atom.Atom(atype='C', xyz=[1/3, 2/3, 0.25], lattice=latt),
         diffpy.structure.atom.Atom(atype='C', xyz=[2/3, 1/3, 0.75], lattice=latt),]
structure_matrix = diffpy.structure.Structure(atoms=atoms, lattice=latt)
calibration = 0.0262
p = Phase("Graphite", point_group="6/mmm", structure=structure_matrix)
euler_angles_new = np.array([[0, 90, 90]])
rot = Rotation.from_euler(euler_angles_new, degrees=True)
rlv = ReciprocalLatticeVector.from_min_dspacing(phase=p, min_dspacing= 1)


rotated_rlv = rlv.rotate_from_matrix(rot.to_matrix()[0])
rlv.hkl ==rotated_rlv.hkl # False

Maybe this will help show the problem related to rotating the RLV.

[CSSFrancis]

@hakonanes have you ever used the https://www.diffpy.org/diffpy.structure/mod_lattice.html baserot parameter. I think that we could have the rotation also rotate the lattice. That would at least allow us to rotate the RLV and be consistent with the hkl values.

[CSSFrancis]

@hakonanes This is partially fixed in pyxem/orix#499. Proper HKL values requires some more thought and better handling.

[hakonanes]

If we override __neg__ in ReciprocalLatticeVector too, this issue is solved.

@viljarjf, I assume you mean that we can override __mul__()? That is a valid solution, although at the cost of code duplication.

Should this even be possible?

@CSSFrancis, good point! Now that I think about it, I'd say no. ReciprocalLatticeVector was initially made to generate the set of strongly scattering reflections for a given unit cell. The intention was never to rotate them. I did this first only in #205 (comment), but was surprised at the result... I thought we subclassed Vector3d from orix perfectly, which we clearly do not.

---

I see you've suggested a change. May I suggest that we instead overwrite __mul__() as @viljarjf suggest, but throw an error explaining that this is not implemented? If we need to rotate the vectors, we can cast them to regular Miller (hkl) first.

[hakonanes]

have you ever used the https://www.diffpy.org/diffpy.structure/mod_lattice.html baserot parameter.

Nope.

I think that we could have the rotation also rotate the lattice. That would at least allow us to rotate the RLV and be consistent with the hkl values.

Hm... But, then what is the point of rotating the vectors in the first place? When I rotate vectors, I always want new vectors.

[CSSFrancis]

@hakonanes So what I've come to is that it should be possible to rotate the RLV as long as we can track the rotation.

This allows us to have consistent hkl values but change the underlying xyz values.

[hakonanes]

Can this be a feature of the private DiffractingVector class in #205 instead?