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.

Sign up for GitHub

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

Property 'fractional_site_positions' #206

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

Property 'fractional_site_positions' #206

Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e7e3792
Defining 'fractional_site_positions' property.
merkys Nov 22, 2019
8aff0e8
Adjusting the description of 'nsites'.
merkys Nov 22, 2019
be8369e
Adjusting description of 'species_at_sites'.
merkys Nov 22, 2019
81cf92b
Adjusting description of 'structure_features'.
merkys Nov 22, 2019
0687fa9
Adjusting the requirement on the presence of 'lattice_vectors' when '…
merkys Nov 22, 2019
4c12f04
Adjusting the description of 'lattice_vectors'.
merkys Nov 22, 2019
fb7c6f3
Making 'lattice_vectors' REQUIRED unless explicitly excluded.
merkys Nov 25, 2019
8eefdd2
Enforcing the same order of 'cartesian_site_positions' and 'fractiona…
merkys Nov 25, 2019
2737e2d
Accepting @rartino's suggestion to clarify relationship between Carte…
merkys Nov 26, 2019
50186bc
Accepting @rartino's suggestion for null values in 'lattice_vectors'
merkys Nov 26, 2019
03f4088
Adjusting the description of dimensions according to the changes to '…
merkys Dec 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions optimade.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1653,15 +1653,16 @@ lattice\_vectors
~~~~~~~~~~~~~~~~

- **Description**: The three lattice vectors in Cartesian coordinates, in ångström (Å).
- **Type**: list of list of floats.
- **Type**: list of list of floats and/or unknown values
- **Requirements/Conventions**:

- **Response**: REQUIRED in the response unless explicitly excluded, except when property `dimension_types`_ is equal to :val:`[0, 0, 0]` (in this case it is OPTIONAL).
- **Response**: REQUIRED in the response unless explicitly excluded, except when property `dimension_types`_ is equal to :val:`[0, 0, 0]` and `fractional_site_positions`_ are not provided (in this case it is OPTIONAL).
merkys marked this conversation as resolved.
Show resolved Hide resolved
- **Query**: Support for queries on this property is OPTIONAL. If supported, filters MAY support only a subset of comparison operators.
- MUST be a list of three vectors *a*, *b*, and *c*, where each of the vectors MUST BE a list of the vector's coordinates along the x, y, and z Cartesian coordinates.
(Therefore, the first index runs over the three lattice vectors and the second index runs over the x, y, z Cartesian coordinates).
- For databases that do not define an absolute Cartesian system (e.g., only defining the length and angles between vectors), the first lattice vector SHOULD be set along *x* and the second on the *xy*-plane.
- This property MUST be an array of dimensions 3 times 3 regardless of the elements of property `dimension_types`_. The vectors SHOULD by convention be chosen so the determinant of the :property:`lattice_vectors` matrix is different from zero. The vectors in the non-periodic directions have no significance beyond fulfilling these requirements.
- If `fractional_site_positions`_ are provided, the vectors in non-periodic directions MUST NOT be expressed as unknown values. Otherwise they MAY be expressed as `[null, null, null]`.
merkys marked this conversation as resolved.
Show resolved Hide resolved

- **Examples**:

Expand All @@ -1688,10 +1689,29 @@ cartesian\_site\_positions

- :val:`[[0,0,0],[0,0,2]]` indicates a structure with two sites, one sitting at the origin and one along the (positive) *z*-axis, 2 Å away from the origin.

fractional\_site\_positions
~~~~~~~~~~~~~~~~~~~~~~~~~~~

- **Description**: Positions of each site expressed as fractions of the `lattice_vectors`_. Description of a site matches the one given in `cartesian_site_positions`_.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is currently formulated, fractional_site_positions and cartesian_site_positions have different status: cartesian is the primary definition, and both species_at_sites and fractional... refer to cartesian.... However, mathematically, cartesian... and fractional... are equivalent descriptions, and either of them or even both may be absent. It then makes this description ambiguous, since, if cartesian_site_positions are absent, it is not clear what the fractional_site_positions must match.

It seems therefore that it would be more consistent to describe species_at_site as a primary definition of an atomic site, and to let both fractional_site_positions and cartesian_site_positions refer to the species_at_site when the order and number of array elements is defined for each fractional and cartesian.

- **Type**: list of list of floats and/or unknown values
- **Requirements/Conventions**:

- **Response**: OPTIONAL in the response.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest specifying that:

`fractional_site_positions` and `cartesian_site_positions` MAY be both present in the same response. 
If they are both present, they SHOULD describe the same points in space, i.e. cartesian coordinates 
of sites should be the same as fractional coordinates of the same sites with orthogonalisation 
convention, as expressed in `lattice_vectors`, applied.

cartesian_site_positions[isite][x,y,z] = 
     fractional_site_positions[isite][x,y,z] * latice_vectors[][];

where:
   -  isite is index of the site (isite = 0..length(species_at_site[])-1);
   -  '*' is understood in matrix multiplcation sense;
   - each lattiec_vectors[i] element (i=0..2) is the 
     corresponding lattice vector, \vec{a}, \vec{b} and \vec{c}.
   - both cartesian_site_positions[isite][x,y,z] and
     fractional_site_positions[isite][x,y,z] are row-vectors
     (not column-vectors).

- **Query**: Support for queries on this property is OPTIONAL. If supported, filters MAY support only a subset of comparison operators.
- It MUST be a list of length N times 3, where N is the number of sites in the structure.
merkys marked this conversation as resolved.
Show resolved Hide resolved
- An entry MAY have multiple sites at the same fractional position (for a relevant use of this, see e.g., the property `assemblies`_).
- If a component of the position is unknown, the :val:`null` value should be provided instead (see section `Properties with unknown value`_).
Otherwise, it should be a float value.
If at least one of the coordinates is unknown, the correct flag in the list property `structure_features`_ MUST be set.

- **Examples**:

- :val:`[[0,0,0],[0.5,0.5,0.5]]` indicates a structure with two sites, one sitting at the origin and one at the center of the unit cell.

nsites
~~~~~~

- **Description**: An integer specifying the length of the :property:`cartesian_site_positions` property.
- **Description**: An integer specifying the length of :property:`cartesian_site_positions` and :property:`fractional_site_positions` properties.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As was pointed out by @sauliusg, this should refer instead only to species_at_sites.

- **Type**: integer
- **Requirements/Conventions**:

Expand All @@ -1710,14 +1730,14 @@ nsites
species\_at\_sites
~~~~~~~~~~~~~~~~~~

- **Description**: Name of the species at each site (where values for sites are specified with the same order of the property `cartesian_site_positions`_).
- **Description**: Name of the species at each site (where values for sites are specified with the same order of the properties `cartesian_site_positions`_ and `fractional_site_positions`_).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- **Description**: Name of the species at each site (where values for sites are specified with the same order of the properties `cartesian_site_positions`_ and `fractional_site_positions`_).
- **Description**: Name of the species at each site (where values for sites are specified with the same order of the properties `cartesian_site_positions`_ as well as `fractional_site_positions`_).

I discourage and, even though it is correct, simply due to the fuzzyness it may present for an implementer in terms of "should it be the sum of these properties then"?

The properties of the species are found in the property `species`_.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest that species_at_sites are moved to the beginning of the specification, and fractional_site_positions and cartesian_size_positions should both refer to the species_at_sites section.

- **Type**: list of strings.
- **Requirements/Conventions**:

- **Response**: REQUIRED in the response unless explicitly excluded.
- **Query**: Support for queries on this property is OPTIONAL. If supported, filters MAY support only a subset of comparison operators.
- MUST have length equal to the number of sites in the structure (first dimension of the list property `cartesian_site_positions`_).
- MUST have length equal to the number of sites in the structure (first dimension of the list properties `cartesian_site_positions`_ and `fractional_site_positions`_).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When species_at_site are moved to the front, the fractional_site_positions should the refer to species_at_sites, not vice versa.

- Each species name mentioned in the :property:`species_at_sites` list MUST be described in the list property `species`_ (i.e. for each value in the :property:`species_at_sites` list there MUST exist exactly one dictionary in the :property:`species` list with the :property:`name` attribute equal to the corresponding :property:`species_at_sites` value).
- Each site MUST be associated only to a single species.
**Note**: However, species can represent mixtures of atoms, and multiple species MAY be defined for the same chemical element.
Expand Down Expand Up @@ -1912,7 +1932,7 @@ structure\_features
- **List of strings used to indicate special structure features**:

- :val:`disorder`: This flag MUST be present if any one entry in the :property:`species` list has a :property:`chemical_symbols` list that is longer than 1 element.
- :val:`unknown_positions`: This flag MUST be present if at least one component of the :property:`cartesian_site_positions` list of lists has value :val:`null`.
- :val:`unknown_positions`: This flag MUST be present if at least one component of the :property:`cartesian_site_positions` and :property:`fractional_site_positions` lists of lists has value :val:`null`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- :val:`unknown_positions`: This flag MUST be present if at least one component of the :property:`cartesian_site_positions` and :property:`fractional_site_positions` lists of lists has value :val:`null`.
- :val:`unknown_positions`: This flag MUST be present if at least one component of the :property:`cartesian_site_positions` or :property:`fractional_site_positions` lists of lists has value :val:`null`.

- :val:`assemblies`: This flag MUST be present if the property `assemblies`_ is present.

- **Examples**: A structure having unknown positions and using assemblies: :val:`["assemblies", "unknown_positions"]`
Expand Down