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

Add pymatgen.io.openff module #3729

Merged
merged 55 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
f52a8df
Add functionality and tests for pymatgen.io.openff.
orionarcher Mar 31, 2024
d360549
Add test files.
orionarcher Apr 4, 2024
d1a9ace
Update testing fixture with correct test file paths.
orionarcher Apr 4, 2024
7328391
Update docstrings to google format.
orionarcher Apr 4, 2024
b9b36b9
Add openff install from source to test.yml
orionarcher Apr 4, 2024
c173c71
Lint with black.
orionarcher Apr 4, 2024
8f5090f
Small formatting change.
orionarcher Apr 4, 2024
2902ea4
Try changing test.yml
orionarcher Apr 4, 2024
d1b7d43
Add manual install of units.
orionarcher Apr 4, 2024
1a2afe4
Add import warning.
orionarcher Apr 4, 2024
8660c1b
Try adding install for openff-utilities.
orionarcher Apr 4, 2024
8eb6b4c
Add openbabel install.
orionarcher Apr 4, 2024
e08161f
Add openbabel dependency to setup.py
orionarcher Apr 4, 2024
09f1e34
Change openbabel version
orionarcher Apr 4, 2024
de88ef8
fix babel dep.
orionarcher Apr 4, 2024
8a7d9c0
Try building babel from source.
orionarcher Apr 5, 2024
8b5528b
Remove openbabel from setup.py and requirements-optional.txt.
orionarcher Apr 5, 2024
f95e2d5
Attempt new testing build with micromamba.
orionarcher Apr 8, 2024
305945c
Remove bader and enumlib
orionarcher Apr 8, 2024
8cf2cc4
Update testing file
orionarcher Apr 8, 2024
1a5ebf2
Try chaning testing to install pytest.
orionarcher Apr 8, 2024
25f44e9
Run testing with micromamba.
orionarcher Apr 8, 2024
d937bca
Fix minor typo.
orionarcher Apr 8, 2024
dc06e5d
Try switching to editable install.
orionarcher Apr 8, 2024
186e365
Try changing TEST_FILES_DIR
orionarcher Apr 8, 2024
b243e2a
Undo previous change
orionarcher Apr 8, 2024
3b88415
Add manual builds of packmol, enumlib, and bader.
orionarcher Apr 8, 2024
0b7b6eb
Switch to openff-toolkit-base
orionarcher Apr 8, 2024
b200a78
Convert manual builds of enumlib, packmol, bader, and openbabel to co…
orionarcher Apr 8, 2024
49bb35f
Try limiting windows openbabel install.
orionarcher Apr 8, 2024
859e83d
Skip openff tests if openff-toolkit not installed
orionarcher Apr 8, 2024
fe1a85a
Try adding uv and skipping tests.
orionarcher Apr 8, 2024
45810d8
Fix typo
orionarcher Apr 8, 2024
25dbd7e
Add non-failing optional import.
orionarcher Apr 8, 2024
040e701
Add importorskip call.
orionarcher Apr 8, 2024
a9bad6b
Fix import or skip and reimport bson.
orionarcher Apr 8, 2024
cf2df32
Reinstall pymongo in CI to fix bson error. Remove commented out manua…
orionarcher Apr 8, 2024
930c801
Fix errors in babel.py
orionarcher Apr 8, 2024
4802525
Remove unneeded step.
orionarcher Apr 8, 2024
ba19c29
fix TestMoleculeGraph.test_construction expected error message on usi…
janosh Apr 9, 2024
7c26ddf
swap assert_array_equal for assert_allclose in test_outputs.py, maybe…
janosh Apr 9, 2024
e07eb56
cast coords to float in BabelMolAdaptor ob_atom.SetVector
janosh Apr 9, 2024
39fe79e
Refactor assert statements in test_outputs.py to use approx() for dicts
janosh Apr 9, 2024
347008e
Coerce formal charge to int in molgraph_to_openff_mol
orionarcher Apr 9, 2024
220af2f
rename micromamba to pmg
janosh Apr 10, 2024
005f610
micromamba activate pmg
janosh Apr 10, 2024
93406b8
remove pymongo install, should not be needed?
janosh Apr 10, 2024
c837d78
micromamba activate pmg in pytest step
janosh Apr 10, 2024
7389f30
set shell: bash -l {0} for mamba env activate
janosh Apr 10, 2024
db4e324
replace hard-to-install bson.BSON (93406b8) with pymatgen-native asse…
janosh Apr 10, 2024
30be59a
replace isinstance with issubclass in assert_msonable
janosh Apr 10, 2024
e023eec
assert_msonable replace obj.to_json() with json.dumps(obj.as_dict(), …
janosh Apr 10, 2024
1410a2e
skip failing TestQCOutput.test_all
janosh Apr 10, 2024
a3e9c17
rename mol(''->_)graph_to_openff_mol
janosh Apr 10, 2024
780394d
missed some
janosh Apr 10, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 15 additions & 43 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,69 +48,41 @@ jobs:
- name: Check out repo
uses: actions/checkout@v4

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
cache: pip
cache-dependency-path: setup.py
- name: Set up micromamba
uses: mamba-org/setup-micromamba@main

- name: Create mamba environment
run: |
micromamba create -n pmg python=${{ matrix.python-version }} --yes

- name: Install uv
run: pip install uv
run: micromamba run -n pmg pip install uv

- name: Copy GULP to bin
if: matrix.os == 'ubuntu-latest'
run: |
sudo cp cmd_line/gulp/Linux_64bit/* /usr/local/bin/

- name: Install Bader
- name: Install ubuntu-only conda dependencies
if: matrix.os == 'ubuntu-latest'
run: |
wget https://theory.cm.utexas.edu/henkelman/code/bader/download/bader_lnx_64.tar.gz
tar xvzf bader_lnx_64.tar.gz
sudo mv bader /usr/local/bin/
continue-on-error: true # This is not critical to succeed.
micromamba install -n pmg -c conda-forge enumlib packmol bader openbabel openff-toolkit --yes
Copy link

Choose a reason for hiding this comment

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

Consider pinning the versions of enumlib, packmol, bader, openbabel, and openff-toolkit to ensure reproducibility and stability in your CI/CD pipeline.

- micromamba install -n pmg -c conda-forge enumlib packmol bader openbabel openff-toolkit --yes
+ micromamba install -n pmg -c conda-forge enumlib=<version> packmol=<version> bader=<version> openbabel=<version> openff-toolkit=<version> --yes

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
micromamba install -n pmg -c conda-forge enumlib packmol bader openbabel openff-toolkit --yes
micromamba install -n pmg -c conda-forge enumlib=<version> packmol=<version> bader=<version> openbabel=<version> openff-toolkit=<version> --yes


- name: Install Enumlib
if: matrix.os == 'ubuntu-latest'
run: |
git clone --recursive https://github.com/msg-byu/enumlib
cd enumlib/symlib/src
export F90=gfortran
make
cd ../../src
make enum.x
sudo mv enum.x /usr/local/bin/
cd ..
sudo cp aux_src/makeStr.py /usr/local/bin/
continue-on-error: true # This is not critical to succeed.

- name: Install Packmol
if: matrix.os == 'ubuntu-latest'
run: |
wget -O packmol.tar.gz https://github.com/m3g/packmol/archive/refs/tags/v20.14.2.tar.gz
tar xvzf packmol.tar.gz
export F90=gfortran
cd packmol-20.14.2
./configure
make
sudo mv packmol /usr/local/bin/
cd ..
continue-on-error: true # This is not critical to succeed.

- name: Install dependencies
- name: Install pymatgen and dependencies
run: |
micromamba activate pmg
# TODO remove temporary fix. added since uv install torch is flaky.
# track https://github.com/astral-sh/uv/issues/1921 for resolution
pip install torch

uv pip install numpy cython --system
uv pip install numpy cython

uv pip install -e '.[dev,optional]' --system
uv pip install -e '.[dev,optional]'

# TODO remove next line installing ase from main branch when FrechetCellFilter is released
uv pip install --upgrade 'git+https://gitlab.com/ase/ase' --system
uv pip install --upgrade 'git+https://gitlab.com/ase/ase'

- name: pytest split ${{ matrix.split }}
run: |
micromamba activate pmg
pytest --splits 10 --group ${{ matrix.split }} --durations-path tests/files/.pytest-split-durations tests
12 changes: 6 additions & 6 deletions pymatgen/io/babel.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def __init__(self, mol: Molecule | openbabel.OBMol | pybel.Molecule) -> None:
ob_atom = openbabel.OBAtom()
ob_atom.thisown = 0
ob_atom.SetAtomicNum(atom_no)
ob_atom.SetVector(*coords)
ob_atom.SetVector(*map(float, coords))
ob_mol.AddAtom(ob_atom)
del ob_atom
ob_mol.ConnectTheDots()
Expand Down Expand Up @@ -182,14 +182,14 @@ def rotor_conformer(self, *rotor_args, algo: str = "WeightedRotorSearch", forcef
else:
self.add_hydrogen()

ff = openbabel.OBForceField_FindType(forcefield)
ff = openbabel.OBForceField.FindType(forcefield)
if ff == 0:
warnings.warn(
f"This input {forcefield=} is not supported "
"in openbabel. The forcefield will be reset as "
"default 'mmff94' for now."
)
ff = openbabel.OBForceField_FindType("mmff94")
ff = openbabel.OBForceField.FindType("mmff94")

try:
rotor_search = getattr(ff, algo)
Expand Down Expand Up @@ -264,10 +264,10 @@ def confab_conformers(
else:
self.add_hydrogen()

ff = openbabel.OBForceField_FindType(forcefield)
ff = openbabel.OBForceField.FindType(forcefield)
if ff == 0:
print(f"Could not find {forcefield=} in openbabel, the forcefield will be reset as default 'mmff94'")
ff = openbabel.OBForceField_FindType("mmff94")
ff = openbabel.OBForceField.FindType("mmff94")
Comment on lines +267 to +270
Copy link

Choose a reason for hiding this comment

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

Replace the print statement with a warning to maintain consistency and provide users with better control over error messages.

- print(f"Could not find {forcefield=} in openbabel, the forcefield will be reset as default 'mmff94'")
+ warnings.warn(f"Could not find {forcefield=} in openbabel, the forcefield will be reset as default 'mmff94'")

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
ff = openbabel.OBForceField.FindType(forcefield)
if ff == 0:
print(f"Could not find {forcefield=} in openbabel, the forcefield will be reset as default 'mmff94'")
ff = openbabel.OBForceField_FindType("mmff94")
ff = openbabel.OBForceField.FindType("mmff94")
ff = openbabel.OBForceField.FindType(forcefield)
if ff == 0:
warnings.warn(f"Could not find {forcefield=} in openbabel, the forcefield will be reset as default 'mmff94'")
ff = openbabel.OBForceField.FindType("mmff94")


if freeze_atoms:
print(f"{len(freeze_atoms)} atoms will be freezed")
Expand Down Expand Up @@ -346,8 +346,8 @@ def from_molecule_graph(cls, mol: MoleculeGraph) -> Self:
"""
return cls(mol.molecule)

@needs_openbabel
@classmethod
@needs_openbabel
def from_str(cls, string_data: str, file_format: str = "xyz") -> Self:
"""
Uses OpenBabel to read a molecule from a string in all supported
Expand Down