pyxem / orix

Analysing crystal orientations and symmetry in Python
https://orix.readthedocs.io
GNU General Public License v3.0
80 stars 48 forks source link

Conventions in orix #249

Open pc494 opened 2 years ago

pc494 commented 2 years ago

Following on from: https://github.com/pyxem/orix/pull/234#issuecomment-988742635

There are lots of conventions needed to work with orientations. We (the developers) are starting to learn that anything other than extremely well documented conventions will lead to problems. This thread aims to spell these conventions out, agree upon them, and also design a strategy for documenting them. I have decided to design it into "Phases" so as not to lead to information overload. Each new phase will be added to the top line to make life easier.

Phase 0 - Setting Terms

And the computer package MTEX (which we will refer to as [4]).

Phase 1

  1. The sample z direction goes perpendicular to the array of our 2D sample (obtained through some scanning mechanism)
  2. The sample basis (and all such basis) should be right handed [1]

Two options jump out: a1. Follow [4], x east, y south, z into page. a2. Stick with what we have, x east, y north, z out of page.

Having decided this we should then decided how the crystal axes for each crystal system 'align' with this basis when no rotation has been applied. This basically involves completing the following table, ideally from a single authoritative source (perhaps noting disagreements with other source as and when needed). Appendix B of [3] suggests this might be a bit tricky. Adjacently, I think this is the cause of https://github.com/pyxem/orix/issues/236

Crystal System e1 e2 e3
Cubic a e3 x e1 c*
Hexagonal a e3 x e1 c*
Tetragonal a e3 x e1 c*
Trigonal a e3 x e1 c*
Orthorhombic a e3 x e1 c*
Monoclinic a e3 x e1 c*
Trigonal a e3 x e1 c*

So questions. q1) a1 or a2? q2) Do we have an authoritative source to complete this table, if not, which ones do we think are obvious and can be agreed upon (eg. cubic is a bit trivial)?

Phase 2

This is more of a code design element. I think we should make almost all from_euler code (and similar) private so that we can maintain a single internal set of conventions that are consistently applied. As an example, I believe the "lab2crystal" and "crystal2lab" arguments set whether imported data is "active" (static if your transform3d) or "passive" (rotating in transform3d) - this can get extremely confusing very quickly. Basically I don't think we want this exposed to users.

In a later phase we will discuss whether we should use "passive" or "active", but this only makes sense if (under the hood) every import is converted into the same form. At present a reasonably careful user can run from_euler and be off by an inversion without anybody really noticing.

(also as a mild addendum to this, I would be much happier if we used axangles in our examples over Eulers, but that's a much more minor point)

Phase 3

It seems we are broadly in agreement on the "passive" convention, in which a rotation R describes the transformation that brings the crystal coordinates into coincidence with the lab coordinates [2]. This has the slightly unwelcome side effect that

R * crystal_vector

is not the correct way to bring some crystal direction from it's "neutral" positions to that described by orientation R instead an inversion is needed.

Would be good to check that everyone is happy with this.

harripj commented 2 years ago

@pc494 great idea to make a separate thread for this.

Two options jump out: a1. Follow [4], x east, y south, z into page. a2. Stick with what we have, x east, y north, z out of page.

I would throw my hat in for a2. This make the most sense to me coming from a TEM background and is consistent with what most people class as xy directions (and of course z out-of-page comes from point 2) and what is currently implemented. However I appreciate that a1 is used in MTEX and I believe makes more sense from an EBSD standpoint.

Appendix B of [3] suggests this might be a bit tricky.

[1] and [3] both seem to suggest the same orthonormal reference frame, namely: e1 = a1 / a, e2 = e3 x e1, e3 = a3* / c*.

hakonanes commented 2 years ago

q1) As long as we're consistent, I think both are OK. The benefit of MTEX' approach is that the two reference frames are always aligned. I'm slightly in favour of a1 for this reason.

q2) Agree with @harripj.

I think we should write down the current conventions in a notebook and add it to the user guide, stating that these can change in the future. Then, when we decide to change something, we can run the notebook to see that we get the expected result, and change it accordingly. I can propose such a notebook based on the snippets and plots in https://github.com/pyxem/orix/pull/234#issuecomment-988136832 after that PR is merged. I suggest to include this notebook and the PR in v0.8, and wait for v0.9 with any changes following the discussion here.

pc494 commented 2 years ago

The benefit of MTEX' approach is that the two reference frames are always aligned. I'm slightly in favour of a1 for this reason.

Could you expand on what this means? Where is the second reference frame coming from?

hakonanes commented 2 years ago

I was a bit unclear here. In MTEX, the sample and Euler reference frames are the same (doc). I realise that (q1) didn't include discussions of the latter. That the axes in MTEX' stereographic projection (PF, IPF etc.) point in the same direction as the sample reference frame further simplifies thinking about reference frames, in my opinion.

pc494 commented 2 years ago

I was a bit unclear here. In MTEX, the sample and Euler reference frames are the same (doc). I realise that (q1) didn't include discussions of the latter. That the axes in MTEX' stereographic projection (PF, IPF etc.) point in the same direction as the sample reference frame further simplifies thinking about reference frames, in my opinion.

Yeah, I think this is a mark for later point. I'm trying to go as slowly as possible so that I don't get jumbled.

pc494 commented 2 years ago

[1] and [3] both seem to suggest the same orthonormal reference frame, namely: e1 = a1 / a, e2 = e3 x e1, e3 = a3* / c*.

Happy with this, but I think it's only half of what we need. In #236 we found that:

lattice=Lattice(5.1505, 5.3173, 5.2116, 90, 90, 99.23)) works (ie. with our current settings) but lattice=Lattice(5.1505, 5.2116, 5.3173, 90, 99.23, 90)) doesn't. This makes sense, and the explanation @hakonanes gives over there is good, but it is probably best if we enumerate all the cases.

eg:

cubic x=a y=b z=c
tetragonal x=a y=b z=c where a=b!=c
hexagonal x=a , y = c ^ a, z=c where gamma = 120

Then the case above becomes something like:

Monoclinic, where beta > 90 (this is the convention in the data book I looked at, and in MTEX, but not in orix right now)

x = a, y = c ^ a, z = c

I think this might be just involve going through the International Tables and clearing things up.

pc494 commented 2 years ago

Okay, going to go away and have a think before trying to start the next phase.

din14970 commented 2 years ago

My two cents:

pc494 commented 2 years ago

One might define an intermediate cartesian coordinate system fixed to the crystal to simplify the issue. (Mis)orientation is then defined by the world and crystal cartesian coordinate system, which no longer depends on the crystal system. How the crystal basis vectors are defined in the fixed cartesian system is then still up for debate; personally I've found it easiest to set a || x, ensure b is in the x-y plane, and c has positive z. This is the convention I used in my own little tool alphabeta and is also used by other tools like dials.

I think this is the approach we will go for, convert everything internally to Cartesians using a known arrangement that limits the choices of a,b and c. eg - A monoclinic cell must be defined to have beta as the non-90 angle.

hakonanes commented 2 years ago

z into the page makes intuitive sense as the direction of the beam

Worth noting that this is not the case for EBSD and off-axis TKD (Transmission Kikuchi Diffraction).

One might define an intermediate cartesian coordinate system fixed to the crystal to simplify the issue.

This is done in the Miller class, which was demonstrated in #236.

pc494 commented 2 years ago

I have added a phase 2 to the OP.

hakonanes commented 2 years ago

I think we should make almost all from_euler code (and similar) private so that we can maintain a single internal set of conventions that are consistently applied.

I agree. No functionality is lost, since the user can just invert the rotation instance if necessary.

I would be much happier if we used axangles in our examples over Eulers

I'm most familiar with Euler angles, thus I always use this, but I agree that definition from an axis/angle pair is more elegant and less error prone. If we implement #250, I'd be happy to change most or all the uses of Orientation.from_euler() to Orientation.from_axes_angles().

pc494 commented 2 years ago

I have now added a phase 3 to the OP

harripj commented 2 years ago

It seems we are broadly in agreement on the "passive" convention, in which a rotation R describes the transformation that brings the crystal coordinates into coincidence with the lab coordinates [2].

I believe this is the opposite of the Bunge convention, which describes a passive rotation of the lab reference frame into the crystal reference frame. This is what is laid out in [1], convention 3. It was my understanding so far that we were going for the latter?

hakonanes commented 2 years ago

I'm more than happy with multiplying by the inverse orientation g^-1 to translate crystal coordinates h into sample coordinates r: r = g^-1 * h. This follows the Bunge convention, but is the opposite of MTEX and Eq. (1) in the clustering paper. This is what we do when we plot a pole figure: select a crystal direction, and rotate it into the sample reference frame by multiplying with the inverse orientation.

hakonanes commented 2 years ago

"passive" convention, in which a rotation R describes the transformation that brings the crystal coordinates into coincidence with the lab coordinates [2]

To clarify, according to [1] the passive/Bunge convention is a transformation from the sample reference frame to the crystal reference frame, is it not?

This is what we do when we plot a pole figure: select a crystal direction, and rotate it into the sample reference frame by multiplying with the inverse orientation.

An example of exactly this is seen in https://github.com/pyxem/orix/pull/234#issuecomment-988136832.

pc494 commented 2 years ago

Bunge (in Texture and Analysis in Material Science) says:

"We describe the orientations ... by specifying the rotation g which transforms the sample fixed coordinate system into the crystal fixed coordinate system"

pc494 commented 2 years ago

Similarly Rowenhorst et al.

"the orientation of the crystal will be described as a passive rotation of the sample reference frame to coincide with the crystal’s standard reference frame"

pc494 commented 2 years ago

Which, I think means we've narrowed down a point of inclarity. MTEX, the orix manuscript (and almost certainly all current orix code) treats an orientation as crystal2lab, meanwhile Rowenhorst et al. and Bunge et al. go the other way.

Refinding https://www.ctcms.nist.gov/~langer/oof2man/RegisteredClass-Bunge.html has reminded me one of the reasons we do this, when working with Euler Angles (bad, don't do this etc.) it is way more natural to use the Rowenhorst/Bunge version as each angle corresponds to a rotation about a crystallographically known axis.

Basically, I think we have the choice now, but it might be good to spend a little while about which operations are easier to keep track of in which conventions. For example:

A * B * v

for rotations A, B and vector v reads like rotate v by B and then by A which (I think) is only true for one of the conventions.

UPD: or similarly:

A^{-1} B

should be the misorientation that takes Orientation A to Orientation B

hakonanes commented 2 years ago

Hm, the references you're giving above to Bunge and Rowenhorst et al. are in line with https://github.com/pyxem/orix/pull/234#issuecomment-988136832, right? Code in that comment use existing orix functionality (and the current status of the unit cell plot PR with inverted rotations).

A B v

The last example of combining rotations in that comment assumes A rotates B, which is in line with Bunge, right?

(and almost certainly all current orix code) treats an orientation as crystal2lab

The examples in the comment (and direct comparison of rotation conversions and quaternion/vector multiplication to the 3Drotations supplementary material to the Rowenhorst paper) tells me the crystal2lab and lab2crystal parameter arguments in from_euler() are mixed up, and that orix is in line with Rowenhorst/Bunge.

pc494 commented 2 years ago

I agree with the comment above. I think [2] might be wrong in it's explanation. The convention would then be that of Rowenhorst/Bunge/https://github.com/pyxem/orix/pull/234#issuecomment-988136832

For the documentation we can write something like:

My only final thought is what we say about passive/active, I (personally) find the terminology needlessly involved for working with two sets of axes (lab and crystal) but am open to thoughts.

pc494 commented 2 years ago

I think this means that orix is currently consistent (although the manuscript about it is incorrect) and we simply had a case of "We (the developers) are starting to learn that anything other than extremely well documented conventions will lead to problems."

hakonanes commented 2 years ago

I agree with the comment above

That's a relief! The list of conventions to write down in the documentation looks good.

I (personally) find the terminology needlessly involved for working with two sets of axes (lab and crystal)

I agree.

I think this means that orix is currently consistent

This is my experience.

"We (the developers) are starting to learn that anything other than extremely well documented conventions will lead to problems."

Big thanks to @harripj for continuing on #234 and to you (@pc494) for raising this issue!

hakonanes commented 2 years ago

We wait to 0.9 for documenting these conventions, right? If so I think #236 must wait as well.

hakonanes commented 2 years ago

Actually, we could do a v0.8.1 with documentation of these conventions, as I don't see any new functionality being added. What do you think?

pc494 commented 2 years ago

I was expecting web style documentation (which can be done in a patch version).

hakonanes commented 2 years ago

I prefer notebooks because then we can explain the conventions with examples and plots. Much like the previous notebook I've listed above with unit cell plots, IPFs and PFs.

pc494 commented 2 years ago

Sorry, yes, I agree, by "web style" I mean stuff that goes up to read-the-full-docs and has a balance of words and code

hakonanes commented 2 years ago

Oh, I misunderstood you and spoke to strongly... Sorry. We fully agree then! Should we have a minimal example of this in v0.8?

pc494 commented 2 years ago

Oh, I misunderstood you and spoke to strongly... Sorry. We fully agree then! Should we have a minimal example of this in v0.8?

Not a problem, I understood you once I engaged my brain!

If you mean a minimal example of the conventions, I would prefer not too until we're sure all the code behaves, if you mean the notebooks, that might be a good idea, although I would lean towards getting something out at this point and maybe patching in the new year if needs be.

hakonanes commented 2 years ago

I agree, let's do it in the new year.

hakonanes commented 2 years ago

Computing a misorientation in orix

@harripj and @pc494 brought this up in #276 (see https://github.com/pyxem/orix/issues/276#issuecomment-1033006202 and newer comments).

A misorientation m12 between o1 and o2 is the rotation bringing o1 into o2, i.e. o1 is the reference orientation. To obtain m12, the canonical syntax in orix is m12 = o2 - o1.

@pc494 suggested to deprecate this syntax, in favor of obtaining the misorientation via a multiplication. @harripj suggested the following syntax m12 = o2 * ~o1, which is in line with the definition in for example "Texture and Anisotropy" by Kocks et al.: gAB = gB * gA^-1.

MTEX can have a similar syntax m12 = inv(o1) * o2 because they keep track of whether an orientation is inverse or not, m-3m -> xyz or xyz -> m-3m, respectively. In order for us to support the suggested syntax, we need to keep track of whether an orientation is from the sample to the crystal, or from the crystal to the sample. We can add this distinction by having a private Orientation._direction attribute which is lab2crystal (default) or crystal2lab, toggle it whenever Orientation.__invert__() is called, and check it when multiplying two orientations: If the two orientations' directions align like so lab2crystal-crystal2lab (o2 * ~o1, from o1 to o2) or crystal2lab-lab2crystal (~o2 * o1, from o2 to o1), return a Misorientation, otherwise return an Orientation.

harripj commented 2 years ago

(~o2 * o1, from o2 to o1)

I don't think that this is the misorientation from o2 to o1:

>>> m21 = ~o2 * o1
>>> (m21 * o2) * ~o1
Orientation (1,) 1
[[-0.0881  0.0051  0.3049 -0.9483]]

I think the correct calculation should be:

>>> m21 = o1 * ~o2
>>> (m21 * o2) * ~o1
Orientation (1,) 1
[[ 1. -0.  0.  0.]]

So whichever direction, o1 to o2 or o2 to o1, the calculation should be to * ~from such that the misorientation * from = to (due to left-side multiplication).

hakonanes commented 2 years ago

So we should simply raise a warning when doing ~o2 * o1?

harripj commented 2 years ago

So we should simply raise a warning when doing ~o2 * o1?

I'm not sure whether there is a use case for it, I haven't come across one yet anyway. Perhaps others have some input on this?

On a separate note regarding the implementation of this if we decide on it, as ~o2 returns a new Orientation instance, this would rely on the ._direction private attribute you suggested?

hakonanes commented 2 years ago

I'm not sure whether there is a use case for it, I haven't come across one yet anyway. Perhaps others have some input on this?

I checked MTEX, which returns a rotation when doing the opposite (to ~from) of their misorientation definition (~to from), so not even returning an orientation

>> ori = orientation.rand(2);
>> inv(ori(1)) * ori(2)
ans = misorientation (1 -> 1)
   Bunge Euler angles in degree
    phi1     Phi    phi2    Inv.
  307.55 106.756 291.778       0
>> ori(1) * inv(ori2))
ans = rotation (show methods, plot)
   Bunge Euler angles in degree
     phi1     Phi    phi2    Inv.
  69.6646 77.0875 65.9812       0

We could do this, i.e. allow multiplication, but the new instance having no crystal symmetry.

On a separate note regarding the implementation of this if we decide on it, as ~o2 returns a new Orientation instance, this would rely on the ._direction private attribute you suggested?

Yes, inverting has to return an orientation specifying a rotation from the crystal to the sample reference frame for the definition to * ~from to work.

harripj commented 2 years ago

I just want to bring a topic up again here which we touched on before but I think needs some clarity. In Rotation.from_euler it is written: https://github.com/pyxem/orix/blob/d474f246fedd1fabc44f65f103db7ee096759a53/orix/quaternion/rotation.py#L416-L420

But from the MTEX docs and also in this thread https://github.com/pyxem/orix/issues/249#issuecomment-991207775 it is written that MTEX should actually be crystal2lab, ie. a transformation from the crystal reference frame to the lab reference frame. What is everyone's opinion about this?

I think this was mentioned before by @hakonanes that these two (lab2crystal and crystal2lab) should be swapped? (I can't find the comment). Alternatively https://github.com/pyxem/orix/issues/249#issuecomment-990877426 suggests that these switches be dropped and we maintain one constant implementation (in which case I would stick with Bunge, ref [2], NIST and MTEX docs, which say that Euler angles in the Bunge convention represent a transform from the lab reference frame to the crystal reference frame).

hakonanes commented 2 years ago

these two (lab2crystal and crystal2lab) should be swapped

Yes, they should be swapped. I think I first mentioned this in https://github.com/pyxem/orix/pull/234#issuecomment-987125129, and the following comments expands upon this a bit.

I suggest we remove the convention argument and swap the direction parameters. We can add a deprecation warning to from_euler() and to_euler() stating these that changes will take effect in 1.0, and release a 0.8.3 patch release?

EDIT: Showing a deprecation warning every time these methods are called is perhaps too much... We can at least warn about this change in the docstrings in a patch release.

pc494 commented 2 years ago

Add a keyword argument with show_warning=True that the user can then change to false as needs be? Then we can also have **kwargs to warn about the deprecation of convention.

More broadly, my feeling is that from_euler is io and so we should probably support both directions, before doing everything internally under the same convention.

harripj commented 2 years ago

I agree that we should get rid of convention.

I must admit that this is confusing me somewhat still, by default Rotation.from_euler() is with direction crystal2lab, this is the MTEX convention- is the default behaviour in orix currently consistent with MTEX (can't currently check on my machine atm)?

By default I think that the direction should be lab2crystal, ie. Bunge, but do we just need to swap the switches or also invert too?

More broadly, my feeling is that from_euler is io and so we should probably support both directions, before doing everything internally under the same convention.

@pc494 I agree, so does having the two directions fulfils this or are there other cases we should also address? Also in the code the direction currently is handled here: https://github.com/pyxem/orix/blob/d474f246fedd1fabc44f65f103db7ee096759a53/orix/quaternion/rotation.py#L450-L451

This reads to me that currently only one direction is processed internally and then the direction is corrected after processing.

pc494 commented 2 years ago

I am pretty sure there are only two "directions" (I think I used "direction" as "active/passive" always felt like a can of worms). If orix tells you that the rotations is:

0.2 radians about (0,0,1)

The "direction" tells you if that rotations takes your lab axes to the crystal axes or visa versa. The two conventions are related by a simple inversion.

If users are working with data correctly imported to orix all results will be based on one convention, which I think it's worth us firmly agreeing on (I imagine it's in the chat above). If they decide to export they can mess around with conventions again as they see fit.

hakonanes commented 2 years ago

Add a keyword argument with show_warning=True that the user can then change to false as needs be? Then we can also have **kwargs to warn about the deprecation of convention.

That's a solution for direct users of orix, yes. It's not a solution for packages, like kikuchipy, which then will emit many warnings.

EDIT: Seems like from_euler() is mostly used in the kikuchipy tests, so there's no problem there.

More broadly, my feeling is that from_euler is io and so we should probably support both directions, before doing everything internally under the same convention.

I agree. I prefer direction over convention.

by default Rotation.from_euler() is with direction crystal2lab, this is the MTEX convention- is the default behaviour in orix currently consistent with MTEX (can't currently check on my machine atm)?

No, default orix returns the opposite of MTEX. What is wrong in orix at the moment is that crystal2lab gives Bunge. We just have to switch how we use the strings, nothing else.

hakonanes commented 2 years ago

I've updated Phase 1 in the top comment with the chosen crystal axes alignment now documented.

hakonanes commented 2 years ago

The only thing I see remaining here is a (notebook) user guide explaining the conventions for operating on vectors and orientations with orientations and misorientations. This guide could be added under the "Orientations" topic.