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

Remove ase/pymatgen/... inputs from constructor in favor of from_ classmethods #10

Closed
mbercx opened this issue Apr 19, 2024 · 1 comment
Assignees

Comments

@mbercx
Copy link
Member

mbercx commented Apr 19, 2024

Currently the constructor of the StructureData still has e.g. the ase input to initialize an instance from an ASE Atoms object:

def __init__(
self,
## RM cell=None,
## RM pbc=None,
ase=None,
pymatgen=None,
pymatgen_structure=None,
pymatgen_molecule=None,
properties: Dict[str, Dict[str, Any]] = {},
**kwargs
): # pylint: disable=too-many-arguments

I would instead want to see a from_ase() class method, e.g.:

from ase.build import bulk

bulk_si = bulk('Si', cubic=True)

structure_data = StructureData.from_ase(bulk_si)
@mikibonacci mikibonacci self-assigned this Apr 19, 2024
@mikibonacci
Copy link
Collaborator

Hi @mbercx , I will work on this 🚀 today

mikibonacci added a commit to mikibonacci/aiida-atomistic that referenced this issue May 8, 2024
This is implemented following the pydantic PR for aiida-core: aiidateam/aiida-core#6255
agoscinski pushed a commit that referenced this issue Jul 17, 2024
This is implemented following the pydantic PR for aiida-core: aiidateam/aiida-core#6255
mikibonacci added a commit that referenced this issue Jul 18, 2024
* Introducing the kind checks.
In principle, we can also not define the kinds, in that case it is up to the plugin
to use the get_kinds() method of the StructureData to define kinds.

* Starting to migrate to mkdocs

* StructureData developer guide,
missing: PropertyCollector and properties.

* First profiling for the atomistic vs orm StructureData generation.
Need more profiling for low-atoms systems, where it seems orm is better

* Comment on profiling

* Additional notes on profilings

* Adding a last scaling test, to be commented and inserted in the github issue

* Adding the automatic kind generation.
Missing: deactivate this automatism.

* Adding the `allow_kinds` input in the StructureData init.
used to deactivate the automatic kind generation.

* Adding the `to_legacy_structuredata` method in atomistc.StructureData

This is provided in temporary way in order to make easier to
migrate the plugin gradually to this new Data.

In a first step, we should use this method anytime the plugin uses
the StructureData. Then add the properties setting.

* - Adding the developer docs
- Adding some bugfixing for the empty StructureData init.

* Added user guide with examples

* Fixing #16

* This solves issue #10

This is implemented following the pydantic PR for aiida-core: aiidateam/aiida-core#6255

* New implementation, following the pydantic PR.

adding the `charges` properties, in the same way as we have the cell and
pbc: hard-coding the getter and setter methods.

For now, no data validation is there.

* Added the support to read charges also from ASE Atoms object.
Added the map_kinds method, which return the mapping for the kinds as
contained in the sites attribute.

* Reverting back to non-pydantic implementation (Just removing the Model
attribute from the StructureData class, nothing else changed)

Added also the charges in the get_ase method.

* Attaching the properties to the Sites.

- code refactored: now `StructureData` and `Site` classes are in separated files, as well as the utils; `Kind` class is removed
- `Site` class now contains the properties: `symbol`, `position`, `mass`, `charge`, `magnetization`, `kind_name`; `kind_name` should be removed. The properties are defined via the `property` decorator
- `from_ase`, `to_ase` methods are added
- `from_pymatgen`, `to_pymatgen` methods are added (**TOBE fixed to add also the magnetization**)
- `from_file`, `to_file` methods are added. They rely on ASE `io.read` and `io.write` functions.
- the `orm.StructureData` (the old one) now has a method `to_atomistic`
- the `utils.py` file contains also the `to_kinds` and `get_kinds` functions, to automatically generate kinds. The user should only use the `get_kinds` function. **TOBE refined: kind names, and remove the other kinds methods from the StructureData class.**
- the `StructureData` constructor now works in this way: we provide the pbc, cell. Then we should use the `append_atom` method afterwards. **TOBE discussed, I guess we cann add lists contaning site-wise value for properties**
- slicing of structure data: defined the __getitem__ method, to obtain a sliced structuredata instance from the initial one.
- TOBE added: `to_supercell` method.

* adding the possiblity to provide lists in the constructor BUT we are
gonna change this by providing the possibility to provide list of sites
(each of them is a dictionary), in the same format as they are
represented in the database

* Moving the get_kinds and _to_kinds (now a private method) into the
StructureData class.

* Removed kind related methods which now are no more needed in the
StructureData

* Utilities for cell and sites update

* New StructureData and StructureDataMutable

both inheriting from StructureDataCore.
Added properties:
- charge
- magmom

Relevant added methods:

- from/to ASE/Pymatgen
- to_legacy
- to_structuredata
- to_mutable_structuredata

Added single page for API tutorial.

Added tests for basic functionalities of both Structure Classes.

* Allowing also magmom to be provided as floats: they
are then stored as [magmom,0,0]

* Improving mutability and minor changes.

Mutability is improved by removing all the setter methods, both in
StructureDataCore and Site classes. Moreover, lists and tuples are
returned as np.array with flags.writeable=False, meaning that we cannot
even modify the internal arrays/lists.

Other minor changes:

- Now in StructureDataCore we have to define the pbc, cell and sites
keywords to initialise it (and so the other subclasses), this is
helpful to then document the inputs.
- `get_kinds` routine fixed to work also with magmoms (arrays)
- docu and tests updated with respect to these changes.

* Adding type hinting and minor fixes

removing mutable attribute in Site class
adding a get_global_properties method in StructureDataCore,
not used now but in the future to have a list of the properties in the
database (not only attached to the single sites), to make easy to query
wrt a property.

* Improving flexibility, automatic get_kinds and pymatgen fixings.

Providing full flexibility in the StructureDataMutable while preserving same data structure of StructureData.

- adding the parent and index when we initialise the Site class (i.e. when we access structure.sites). This is helpful to be able to change directly properties in the StructureDataMutable class.
- in the same direction, we implemented the ObservedArray class which basically allows to trigger the setter methods of arrays/lists even if we modify only one element (for example: structure.sites[0].position[0] = 5). In this way, changes are written into the structure._data, which represents the real data stored in the instance.

`get_kinds` method now is stable.

Fixing pymatgen problems with oxi_state and magmom.

* Adding in the tutorial markdown also the get_kinds method, and minimal
update of the pytests.
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

No branches or pull requests

2 participants