Skip to content

Deprecate top-level cutoff, always deduce it from bonds::rcut #104

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.

Sign up for GitHub

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

Merged
yury-lysogorskiy merged 3 commits into from
Aug 27, 2026
Merged

Deprecate top-level cutoff, always deduce it from bonds::rcut #104

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f366916
Deprecate top-level `cutoff`, always deduce it from bonds::rcut
yury-lysogorskiy Aug 27, 2026
3bfa4df
CI: install lib/maxvolpy explicitly so the test suite can be collected
yury-lysogorskiy Aug 27, 2026
d63466f
tests: import UnitCellFilter from ase.filters, falling back to ase.co…
yury-lysogorskiy Aug 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
CI: install lib/maxvolpy explicitly so the test suite can be collected 
Both CI jobs aborted during pytest collection with

    pyace/activelearning.py:9: from maxvolpy.maxvol import maxvol
    E ModuleNotFoundError: No module named 'maxvolpy'
    Interrupted: 2 errors during collection

so no test ran at all. This is pre-existing and unrelated to any particular
change - run 31264271622 (17 Aug, PR #103) fails identically.

`maxvolpy` is installed by the custom InstallMaxVolPyLocalPackage(install)
command in setup.py, which shells out to `cd lib/maxvolpy; python setup.py
install`. The workflow installs with `pip install .`, which builds a wheel and
never invokes the legacy `install` command, so that hook never runs.
(`python setup.py install` is also gone in setuptools >= 80.)

Build it explicitly instead. `--no-build-isolation` is required because
lib/maxvolpy/setup.py imports Cython and numpy at module level while declaring
no pyproject.toml build-system, so an isolated build environment fails with
ModuleNotFoundError: No module named 'Cython'. Cython and scipy are installed
first: the extension cimports scipy/linalg/cython_blas.pxd. numpy is
deliberately not installed here, to avoid resolving past setup.py's
`numpy<=1.26.4` pin; it is already present from the python-ace install.

Verified locally in a clean venv: with build isolation the build fails on the
missing Cython, without scipy it fails cythonizing _maxvol.pyx, and with the
command used here it builds a wheel and `from maxvolpy.maxvol import maxvol`
succeeds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017QbfG2UVKxkeuiz9GtDLo4
  • Loading branch information
@yury-lysogorskiy @claude
yury-lysogorskiy and claude committed Aug 27, 2026
commit 3bfa4dfdfb6263332be6ecffe6968f8629f90bec
10 changes: 10 additions & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,16 @@ jobs:
if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
#python setup.py install
pip install .
- name: Install maxvolpy
# `pip install .` builds a wheel and never runs setup.py's custom
# InstallMaxVolPyLocalPackage(install) hook, so lib/maxvolpy is not
# installed and the active-learning tests fail to import. Build it
# explicitly; --no-build-isolation is required because
# lib/maxvolpy/setup.py imports Cython and numpy at module level
# without declaring them in a pyproject.toml build-system.
run: |
pip install Cython scipy
pip install --no-build-isolation ./lib/maxvolpy
- name: Test with python-ace with tensorpotential
run: |
python -m pytest tests/ --runtensorpot
Expand Down
Loading