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

Make pymatgen optional #66

Open
davidwaroquiers opened this issue Nov 20, 2024 · 0 comments
Open

Make pymatgen optional #66

davidwaroquiers opened this issue Nov 20, 2024 · 0 comments

Comments

@davidwaroquiers
Copy link
Member

Opening this issue regarding the removal of pymatgen as a required dependency.

The reason for doing this has been discussed at the Turbomole workshop in Oxford, as well as with BASF. In order to get the community to use and possibly contribute to turbomoleio, such a change is needed. Indeed, pymatgen is seldom used by quantum chemists and has many dependencies, which often make other quantum chemistry-related python packages incompatible with some underlying dependencies of pymatgen (in addition to the amount of packages installed by itself and associated storage size).

The idea is to be able to use turbomoleio without pymatgen. pymatgen would still be an optional dependency. There will be some backward incompatible changes as the main objects describing systems (MoleculeSystem and PeriodicSystem) will not be based on pymatgen anymore. There should be helper functions, available when pymatgen is installed, that can initialize the new MoleculeSystem/PeriodicSystem representation from pymatgen objects.

Here below are the things that need to be done.

src/turbomoleio
	core
		__init__.py
		base.py # has to be modified
		control.py # very small change
		datagroups.py
		molecule.py # has to be modified
		periodic.py # has to be modified
		symmetry.py
		utils.py
	output
		__init__.py
		data.py # has to be modified
		files.py
		parser.py
		states.py

The file control.py uses pymatgen only for a utility decorator function for matplotlib's keyword arguments
The file base.py defines

  • BaseSystem, which is the common base class for both periodic and non-periodic systems
    • It has a Molecule or Structure as an argument
      • This should be modified to be built-in types instead that represents coordinates, lattice, …
      • We should add a from_pymatgen (or another name) class method that only works if pymatgen is installed
      • The to_file method should be adapted (maybe if pymatgen or something else is installed, it could deal with multiple file formats, otherwise just remove it as there is to_coord_file)
  • A method to get the molecule and frozen indices from a coord datagroup
    • Should be modified according to what is chosen as the arguments in BaseSystem
      The files molecule.py/periodic.py use units conversion from pymatgen and Molecule/Structure
  • Should be changed according to base.py/BaseSystem
    The file data.py (and by extension files.py as it uses data objects from data.py) may need to be adapted or we keep them like that and they can only be used if pymatgen is installed.

For the rest, tests will of course need to be updated.
Maybe, we could think of changing the versioning scheme at the same time to have a matching between turbomoleio versions and Turbomole versions ? We could have turbomoleio M.X.y matching Turbomole M.X (but not the minor TM version), the "y" being new features AND bug fixes for a given version of TM. Any thoughts @gpetretto ?

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

1 participant