Fix: No search for MLatom in the virtual environment#40
Conversation
Signed-off-by: Christoph Plett <plett@faccts.de>
stgeo
left a comment
There was a problem hiding this comment.
Thanks for the fix! The changes look OK to me (see one comment below).
I tried a fresh install with
./install.py --venv-dir venv-mlatom --extra mlatom -dand got a couple of issues when running the tests:
- With the venv activated
python test_mlatom.pythrows (in addition to the same warnings as below
FileNotFoundError: [Errno 2] No such file or directory: PosixPath('/redacted/orca-external-tools/venv-mlatom/lib/python3.12/site-packages/oet/../../bin/oet_mlatom')
Looks like ROOT_DIR is not what we expect.
2. Replacing ROOT_DIR with Path(__file__).resolve().parent I get
/redacted/orca-external-tools/venv-mlatom/lib/python3.12/site-packages/torchani/aev.py:16: UserWarning: cuaev not installed
warnings.warn("cuaev not installed")
/redacted/orca-external-tools/venv-mlatom/lib/python3.12/site-packages/torchani/__init__.py:39: UserWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html. The pkg_resources package is slated for removal as early as 2025-11-30. Refrain from using this package or pin to Setuptools<81.
from pkg_resources import get_distribution, DistributionNotFound
/redacted/orca-external-tools/venv-mlatom/lib/python3.12/site-packages/torchani/__init__.py:55: UserWarning: Dependency not satisfied, torchani.ase will not be available
warnings.warn("Dependency not satisfied, torchani.ase will not be available")
Checking the model files and downloading them if necessary.
/redacted/orca-external-tools/venv-mlatom/lib/python3.12/site-packages/torchani/resources/
F..
======================================================================
FAIL: test_H2O_engrad (__main__.MLatomTests.test_H2O_engrad)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/redacted/orca-external-tools/tests/mlatom/test_mlatom.py", line 104, in test_H2O_engrad
self.assertAlmostEqual(energy, expected_energy, places=9)
AssertionError: -76.38342071073 != -76.38342071002 within 9 places (7.099885124262073e-10 difference)
----------------------------------------------------------------------
Ran 3 tests in 14.001s
FAILED (failures=1)
The deviation might be due to the mlatom update (BTW please mention the version bump in the commit message and/or PR description).
We really should integrate the tests into the CI...
I guess the torchANI/CUAEV installation is more complicated - we may want to check if we can automatically get a system-specific setup or at least expose the configuration options to the user at install time. Nothing to do now though.
Signed-off-by: Christoph Plett <plett@faccts.de>
Signed-off-by: Christoph Plett <plett@faccts.de>
|
@stgeo Thanks for the review. Regarding 1: The problem with Regarding 2: I lowered the precision for the checks to 10^-7 as 10^-9 seemed to be a little bit to strict. |
stgeo
left a comment
There was a problem hiding this comment.
LGTM, thanks for looking into the issues!
|
|
||
| print("Installing developer tools.") | ||
| subprocess.check_call([pip_path, "install", ".[dev]"]) | ||
| subprocess.check_call([pip_path, "install", "-e", ".[dev]"]) |
There was a problem hiding this comment.
It's not ideal if the tests only work for an editable install. We probably need a different strategy to find the binaries than using ROOT_DIR. Please open an issue or add it to the requirements for the restructuring of the test suite.
$PATH, the respectiveoet_mlatomscript didn't work, even though MLatom was available in the installed virtual environment. With this fix, the MLatom executable is now searched also for in the respective virtual environment.devfeatures during the setup of the virtual environment is now done in editable mode similar to the standard installation.