StructureData discussion

[mikibonacci]

This was the discussion during a first internal demo of the new StructureData.

Two classes for StructureData . Namings

The first concern is about the two twin classes, StructureData and StructureDataMutable.
The naming should be discussed more, in particular for StructureDataMutable. A solution could be to just let the user work with the StructureDataMutable and then automatically convert/serialize into the AiiDA StructureData. However, when the user try to load/query a StructureData node, this is the immutable one.

Should we rename to be StructureDataNode the immutable and StructureData the mutable? In this way we know that one is really an AiiDA node and cannot change. This would be consistent (somehow) with the definition of Process and ProcessNode (stored in the db) - to be discussed more (see future AEP by @edan-bainglass ).

More about names, the "direct" properties should be renamed "global".

Update (October 2nd 2024)

Now, we have no more "direct"/"global" properties, we dropped the distinction for simplicity and because this information was not actually used anywhere.

[mikibonacci]

Duplication of the information in the AiiDA database

This other issue is coming from the fact that indeed in the database we store both the sites as list of dictionaries (with all the properties), both the property as single lists.
Indeed, structure.base.attributes.all will give us:

{'pbc': [True, True, True],
 'cell': [[0.0, 1.8, 1.8], [1.8, 0.0, 1.8], [1.8, 1.8, 0.0]],
 'custom': None,
 'sites': [{'symbol': 'Cu',
   'kind_name': 'Cu0',
   'position': [0.0, 0.0, 0.0],
   'mass': 63.546,
   'charge': 1.0,
   'magmom': [0.0, 0.0, 0.0],
   'weights': (1,)}],
 'cell_volume': 11.664000000000001,
 'dimensionality': {'dim': 3, 'label': 'volume', 'value': 11.664000000000001},
 'charges': [1.0],
 'magmoms': [[0.0, 0.0, 0.0]],
 'masses': [63.546],
 'kinds': ['Cu0'],
 'symbols': ['Cu'],
 'positions': [[0.0, 0.0, 0.0]],
 'formula': 'Cu'}

So there is a duplication of information (e.g. chargesare both a list and both defined for each site), and also the risk of having inconsistencies (due to bugs). Even if we can ensure consistency with tests (and the fact that the lists are actually computed_fields from the sites properties), we have anyway duplication.
This can be solved by interchanging the role of computed_fields and putting the sites to be computed on the fly with respect to the properties lists.
This change a lot how the model is implemented, but is reasonable. See the below point for more discussion on the resulting API.

Update (October 2nd 2024) - SOLVED

Now, the computed_fields are the sites, so these are not stored in the database. Now, the result of structure.base.attributes.all will be:

{'pbc': [True, True, True],
 'cell': [[0.0, 1.8, 1.8], [1.8, 0.0, 1.8], [1.8, 1.8, 0.0]],
 'cell_volume': 11.664000000000001,
 'dimensionality': {'dim': 3, 'label': 'volume', 'value': 11.664000000000001},
 'charges': [1.0],
 'magmoms': [[0.0, 0.0, 0.0]],
 'masses': [63.546],
 'kinds': ['Cu0'],
 'symbols': ['Cu'],
 'positions': [[0.0, 0.0, 0.0]],
 'formula': 'Cu'}

i.e. no duplication of the information. The sites property is still under the properties attribute of the instance of the node, but it cannot be modified directly (is a computed field, indeed) -> only via dedicated methods (e.g. update_site).

---

[mikibonacci]

Input parameters for the constructor. Two choices.

The implemented way of providing the inputs is by means of a dictionary where the properties are actually attached to each site, so having a list of dictionaries, each of them corresponding to a site. This is also done because we initialise and store the model in the same format, in the AiiDA database.
However, as suggested, we should think also on provide the possibility to defined lists of properties:

structure_dict = {
'pbc':[True,True,True],
'symbols':["Cu","Cu"],
'positions':[...],
'charges':[1,-1],
...
}

In this way, we then generated the sites as computed_fields of the model. This is fine, but actually can be a bit annoying to defined a full list with len(property)==len(symbols). To elaborate more on that, suppose you have ten atoms but only one of them has a charge. Should you defined a full list of [0,0,0,0,1,0,0...], supposing zero is the default value? How the user will know the default value of a given property? We are forcing the user to define the property for each site, even if the property should be attached to only one site.

Update (October 2nd 2024) - IN PROGRESS

Now we support both type of inputs for the constructor, i.e. both providing a list of sites with the properties for each sites, both providing properties as lists, each element corresponding to a site (as described above).
missing (but will be implemented soon): making this two ways mutually exclusive for structure generation (i.e. if sites are provided, you cannot provide also the single properties in lists, and viceversa).

---

[mikibonacci]

Custom properties

There is the risk that someone will name the custom properties as the supported ones. This is an experimental feature only for the pilot phase, then we should somehow provide a protocol to do a PR to aiida-atomistic to add officially supported properties.

---

[mikibonacci]

Kinds generation cannot happen on-the-fly after calcjob submission

This is because for different versions of the plugins (and so maybe different kind thresholds) we can then have different resulting structures. This can create inconsistencies.

[mikibonacci]

Should we move some of the properties in files?

If we don't need (or we cannot) to query them, yes.

[mikibonacci]

Query

This is crucial, we need to understand if we can efficiently query for a structure. This determines also what we will store in the db.
After a brainstorming, these can be the possible queries:

• is alloy?
• contains a given element, contains only a given element?
• number of kinds, sites?
• binary, ternary? or count the number of elements?
• magnetic or not?
• charge defined?
• in general, we have a given property or not?
• magnetic moment per cell?

what about querying inner lists? for SQLAlchemy should be possible, for SQLite?