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

Format of 'lattice_vectors' is unclear for 2D, 1D and 0D systems #196

Closed
merkys opened this issue Nov 7, 2019 · 6 comments · Fixed by #231 · May be fixed by #206
Closed

Format of 'lattice_vectors' is unclear for 2D, 1D and 0D systems #196

merkys opened this issue Nov 7, 2019 · 6 comments · Fixed by #231 · May be fixed by #206
Assignees
Milestone

Comments

@merkys
Copy link
Member

merkys commented Nov 7, 2019

Specification defines lattice_vectors as three lattice vectors in Cartesian coordinates. However, it is not clear how the vectors are defined for nonperiodic dimensions. Currently it seems that such vectors are ignored, and as such can contain any values. But it would probably make sense to have a default, and I would suggest having nulls for all the coordinates of vectors in nonperiodic dimensions.

An example of a 2D surface on XZ plane:

...
  "dimension_types": [1, 0, 1],
  "lattice_vectors": [[4, 0, 0], [null, null, null], [0, 0, 4]],
...
@rartino rartino added this to the 1.0 release milestone Nov 8, 2019
@rartino
Copy link
Contributor

rartino commented Nov 8, 2019

Do you think it is OK to merely suggest nulls, but allow any number? (Preferably not even "SHOULD be null", but "MAY be null".)

I was a somewhat strong proponent for standardizing on reduced coordinates. Hence, I will certainly provide an omdb_reduced_site_positions alongside the standard cartesian_site_positions. In reduced coordinates, it makes sense to have lattice vectors also for the non-periodic coordinates, they act as multipliers for the coordinates in the omdb_reduced_site_positions list, so, for me, those values are going to be meaningful.

@merkys
Copy link
Member Author

merkys commented Nov 11, 2019

Non-null values indeed make sense having considered the reduced coordinates. Is there an issue open for their inclusion? Maybe we could get reduced_site_positions added before v1.0 release?

@rartino
Copy link
Contributor

rartino commented Nov 18, 2019

No issue as far as I know. I've only planned it as a database-specific property. Unless multiple databases chime in an say it is very useful for them, we can standardize it next meeting.

@merkys
Copy link
Member Author

merkys commented Nov 18, 2019

I'd suggest either to:

  1. Standardize reduced_site_positions for v1.0 and require non-null values when reduced_site_positions is present;
  2. Leave reduced_site_positions for v1.1 and discourage non-null values for nonperiodic vectors until then.

What do you think?

@rartino
Copy link
Contributor

rartino commented Nov 18, 2019

I say, 2, but not so much discourage non-null values as saying that values MAY be null.

But if someone wants to do 1, I'll be happy to merge that PR.

@merkys
Copy link
Member Author

merkys commented Nov 22, 2019

But if someone wants to do 1, I'll be happy to merge that PR.

I have opened PR #206.

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

Successfully merging a pull request may close this issue.

2 participants