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

Miller warning about lack of Structure in Phase #439

Open
anderscmathisen opened this issue Mar 16, 2023 · 5 comments
Open

Miller warning about lack of Structure in Phase #439

anderscmathisen opened this issue Mar 16, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@anderscmathisen
Copy link
Contributor

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 hakonanes added the enhancement New feature or request label Mar 16, 2023
@hakonanes hakonanes added this to the v0.12.0 milestone Mar 16, 2023
@hakonanes
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Collaborator

harripj commented Mar 16, 2023

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
Copy link
Member

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
Copy link
Member

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.

@hakonanes hakonanes removed this from the v0.12.0 milestone Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants