Skip to content
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

Make numpy-quaternion an optional dependency #514

Closed
Tracked by #518
hakonanes opened this issue Sep 4, 2024 · 4 comments · Fixed by #519
Closed
Tracked by #518

Make numpy-quaternion an optional dependency #514

hakonanes opened this issue Sep 4, 2024 · 4 comments · Fixed by #519
Labels
dev Package maintenance
Milestone

Comments

@hakonanes
Copy link
Member

I see numpy-quaternion has become a NumPy-2.0-transition-bottleneck downstream for pyxem (pyxem/pyxem#1099). There's discussion of our use of this package in #506 and #507.

I suggest to make numpy-quaternion an optional dependency.

We use numpy-quaternion for three operations:

  • Conjugation:
    @property
    def conj(self) -> Quaternion:
    r"""Return the conjugate of the quaternion
    :math:`Q^* = a - bi - cj - dk`.
    """
    Q = quaternion.from_float_array(self.data).conj()
    return self.__class__(quaternion.as_float_array(Q))
  • quaternion-quaternion multiplication:
    if isinstance(other, Quaternion):
    Q1 = quaternion.from_float_array(self.data)
    Q2 = quaternion.from_float_array(other.data)
    return other.__class__(quaternion.as_float_array(Q1 * Q2))
  • quaternion-vector multiplication:
    elif isinstance(other, Vector3d):
    # check broadcast shape is correct before calculation, as
    # quaternion.rotat_vectors will perform outer product
    # this keeps current __mul__ broadcast behaviour
    Q1 = quaternion.from_float_array(self.data)
    v = quaternion.as_vector_part(
    (Q1 * quaternion.from_vector_part(other.data)) * ~Q1
    )

All places for speed, not for functionality. We had implementations of these things without numpy-quaternion before. We can re-introduce these computations as a fall-back if numpy-quaternion is unavailable.

As an added bonus to this work, I want to make it easier to make dependencies in orix optional. numpy-quaternion is a good place to start.

We can do this fairly quickly. If interest, we can make a v0.13.1 patch release.

@hakonanes hakonanes added the dev Package maintenance label Sep 4, 2024
@hakonanes hakonanes added this to the v0.13.1 milestone Sep 4, 2024
@hakonanes hakonanes changed the title Make numpy-quaternion and optional dependency (for speed) Make numpy-quaternion an optional dependency (for speed) Sep 5, 2024
@hakonanes hakonanes changed the title Make numpy-quaternion an optional dependency (for speed) Make numpy-quaternion an optional dependency Sep 5, 2024
@hakonanes
Copy link
Member Author

I've gotten one 👍🏻, so I'm going for it

@CSSFrancis
Copy link
Member

@hakonanes That would be fantastic! I think @ericpre would also be very happy :)

@pc494
Copy link
Member

pc494 commented Sep 6, 2024

I certainly will be very happy. I assume the speed trade-off is significant, but the documentation can be marked up to reflect that.

@CSSFrancis
Copy link
Member

I certainly will be very happy. I assume the speed trade-off is significant, but the documentation can be marked up to reflect that.

I think it's a factor of 2 is what @hakonanes said. I feel that is pretty minimal, there are other places in orix where it would be easier to speed things up if we really needed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev Package maintenance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants