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

`Miller` warning about lack of `Structure` in `Phase` #439

Open anderscmathisen opened 1 year ago

anderscmathisen commented 1 year ago

A few times now I have been puzzled by Miller giving "wrong" angles between vectors. The issue every time is that I forget to add a Structure argument to the Phase.

from diffpy.structure import Lattice, Structure
from orix.vector import Miller
import numpy as np

a = 0.6115
c = 1.141

lattice = Lattice(a, a, c, 90,90,120)
phase_with_structure = Phase("6/mmm", 191, structure=Structure(lattice=lattice))
phase_no_structure = Phase("6/mmm", 191)

V1 = Miller(UVTW=[2,-1,-1,1], phase=phase_with_structure)
V2 = Miller(UVTW=[2,-1,-1,1], phase=phase_no_structure)

print(np.rad2deg(V1.angle_with(Miller(UVTW=[0,0,0,1], phase=phase_with_structure)))) # out: 58.120
print(np.rad2deg(V2.angle_with(Miller(UVTW=[0,0,0,1], phase=phase_no_structure)))) #out: 71.57

Should we add a warning when creating Miller objects (or perhaps the Phaseobject) that if a Phase with no Structure is passed, then calculations might not be correct? (at least for non cubic systems)

hakonanes commented 1 year ago

Yes, having a correct structure (with a lattice) is a responsibility of the user. The documentation states that if a structure is not passed to Phase, a default is used. The default is a default diffpy.structure.lattice.Lattice with (a, b, c, alpha, beta, gamma) = (1, 1, 1, 90, 90, 90), regardless of symmetry. Changing this is on my orix todo list, but I don't have time at the moment.

The solution I see is that when a lattice is not given, we set it to (1, 1, 1, 90, 90, 90) unless the symmetry given belongs to the hexagonal or trigonal crystal system, in which case we use (1, 1, 1, 90, 90, 120).

anderscmathisen commented 1 year ago

The solution I see is that when a lattice is not given, we set it to (1, 1, 1, 90, 90, 90) unless the symmetry given belongs to the hexagonal or trigonal crystal system, in which case we use (1, 1, 1, 90, 90, 120).

This is a good solution, but I think a warning printed to the user might still be beneficial as for instance for the hexagonal systems, calculations such as angles are dependent on the a/c ratio.

harripj commented 1 year ago

I think in this case another option could be to have no default structure and require the user to define it, if it is easily forgotten and leads to incorrect results.

What might help is if the docstring for Phase includes some basic examples, eg. cubic and hexagonal phase setups, and also if it explicitly specifies that the default structure is cubic without referring the user to diffpy. What do you think to this?

hakonanes commented 1 year ago

I think a warning printed to the user might still be beneficial as for instance for the hexagonal systems, calculations such as angles are dependent on the a/c ratio.

I agree that it is useful for a user to be warned when they create a Miller instance with an hexagonal/trigonal Phase with an invalid lattice. In this case a user is specifically looking to analyze crystal directions or plane normals. I think a warning in this case might be suitable in every situation, actually... What I'd like to avoid is a warning being raised when the user does not care about the a/c ratio.

What might help is if the docstring for Phase includes some basic examples, eg. cubic and hexagonal phase setups, and also if it explicitly specifies that the default structure is cubic without referring the user to diffpy. What do you think to this?

Yes, initialization of a Phase instance should be made simpler. What I'd like to add is class methods to create a base Phase for each crystal system allowing the necessary lattice parameters (and corresponding symmetry, allowing to set this and a structure if desirable). These could then be used in the docstring.

hakonanes commented 1 year ago

specifies that the default structure is cubic without referring the user to diffpy

Yes, we should specify that the default is (1, 1, 1, 90, 90, 90) unless trigonal/hexagonal.