orix icon indicating copy to clipboard operation
orix copied to clipboard

Refactor the "random" method of Rotations (low-priority)

Open pc494 opened this issue 4 years ago • 2 comments

It currently uses rejection based sampling, I would prefer the method described in:

https://mathworld.wolfram.com/HyperspherePointPicking.html

pc494 avatar Aug 05 '21 17:08 pc494

I haven't used Rotation.random(), does it not produce the results you would like, is it inefficient, or both?

https://github.com/pyxem/orix/blob/05311f6a5623453a8f50a9308066d2a3e66f6d29/orix/quaternion/rotation.py#L531-L547

I'm not entierly sure how to implement the equations in the link, but I guess you do. The hypersphere point picking should be added as an alternative method to the existing one, I assume.

hakonanes avatar Aug 06 '21 09:08 hakonanes

The current version is correct, it's doing the Marsaglia (1972) method from:

https://mathworld.wolfram.com/SpherePointPicking.html

which gives the correct answer. However it's slower than the version I'll replace it with because it involves discarding entries (also it's a while loop).

Consider this nothing more than house keeping that came up while I was working with some of these bits of the code. I'll implement the fix in my big bundle of things.

pc494 avatar Aug 06 '21 09:08 pc494