Skip to content

Feat/multipackmol change box shape #448

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

Open
volneu wants to merge 4 commits into
base: main
Choose a base branch
from
+131 −3
Open

Feat/multipackmol change box shape #448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
59ccfde
multipackmol now can change the box shape
Feb 15, 2026
c528916
switch Packmol.run() to use molify.pack with ratio
Feb 16, 2026
8e19bff
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 16, 2026
8ff3969
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 19, 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
15 changes: 12 additions & 3 deletions ipsuite/configuration_generation/packmol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
import ase.units
import h5py
import numpy as np
import rdkit2ase
import znh5md
import zntrack
from molify.pack import pack

from ipsuite import base

Expand Down Expand Up @@ -52,6 +52,7 @@ class Packmol(base.IPSNode):
density: float = zntrack.params()
pbc: bool = zntrack.params(True)
frames_path: pathlib.Path = zntrack.outs_path(zntrack.nwd / "frames.h5")
ratio: list[float] = zntrack.params(default_factory=lambda: [1.0, 1.0, 1.0])

def __post_init__(self):
if len(self.data) != len(self.count):
Expand All @@ -66,12 +67,14 @@ def run(self):
data = self.data

frames = [
rdkit2ase.pack(
# rdkit2ase.pack(

@PythonFZ PythonFZ Feb 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove comments

pack(
data=data,
counts=self.count,
tolerance=self.tolerance,
density=self.density,
pbc=self.pbc,
ratio=self.ratio,
packmol=os.environ.get("RDKIT2ASE_PACKMOL", "packmol"),
)
]
Expand Down Expand Up @@ -129,6 +132,10 @@ class MultiPackmol(Packmol):
seed: int = zntrack.params(42)

def run(self):
# print("RATIO IN NODE:", self.ratio)
# print("PACK FUNCTION:", rdkit2ase.pack)
# print("MODULE:", rdkit2ase.pack.__module__)
# print(rdkit2ase.__file__)
Comment on lines +135 to +138

@PythonFZ PythonFZ Feb 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove comments

np.random.seed(self.seed)
frames = []
for _ in range(self.n_configurations):
Expand All @@ -139,12 +146,14 @@ def run(self):
data.append(frame_list)

frames.append(
rdkit2ase.pack(
# rdkit2ase.pack(
pack(
data=data,
counts=self.count,
tolerance=self.tolerance,
density=self.density,
pbc=self.pbc,
ratio=self.ratio,
packmol=os.environ.get("RDKIT2ASE_PACKMOL", "packmol"),
)
)
Expand Down
119 changes: 119 additions & 0 deletions test_pack.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
from unittest.mock import MagicMock, patch

import ase
import pytest
Comment on lines +1 to +4

@PythonFZ PythonFZ Feb 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In IPSuite integration tests should use the testing procedure a user would use.
See https://github.com/zincware/IPSuite/blob/main/tests/integration/test_smiles_to_atoms.py


from ipsuite import MultiPackmol, Packmol

# -----------------------------
# Helpers
# -----------------------------


def dummy_atoms():
return ase.Atoms("H2")


# -----------------------------
# Basic init tests
# -----------------------------


def test_packmol_ratio_default():
node = Packmol(
data=[[dummy_atoms()]],
count=[1],
density=1000,
)
assert node.ratio == [1.0, 1.0, 1.0]


def test_packmol_ratio_custom():
node = Packmol(
data=[[dummy_atoms()]],
count=[1],
density=1000,
ratio=[1.0, 2.0, 3.0],
)
assert node.ratio == [1.0, 2.0, 3.0]


def test_packmol_data_count_mismatch():
with pytest.raises(ValueError):
Packmol(
data=[[dummy_atoms()], [dummy_atoms()]],
count=[1], # mismatch
density=1000,
)


# -----------------------------
# Mutable default safety
# -----------------------------


def test_ratio_not_shared_between_instances():
a = Packmol(
data=[[dummy_atoms()]],
count=[1],
density=1000,
)
b = Packmol(
data=[[dummy_atoms()]],
count=[1],
density=1000,
)

a.ratio[0] = 99.0
assert b.ratio == [1.0, 1.0, 1.0]


# -----------------------------
# run() call test (mock pack)
# -----------------------------


@patch("ipsuite.pack.pack")
@patch("ipsuite.pack.znh5md.IO")
def test_packmol_run_calls_pack(mock_io, mock_pack):
mock_pack.return_value = dummy_atoms()
mock_io.return_value = MagicMock()

node = Packmol(
data=[[dummy_atoms()]],
count=[1],
density=1000,
ratio=[1.0, 2.0, 1.0],
)

node.run()

mock_pack.assert_called_once()
kwargs = mock_pack.call_args.kwargs
assert kwargs["ratio"] == [1.0, 2.0, 1.0]
Comment on lines +76 to +93

@coderabbitai coderabbitai Bot Feb 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how pack is imported and used in the production packmol module
echo "=== Finding packmol.py ==="
fd packmol.py --type f

echo ""
echo "=== Contents of packmol.py ==="
fd packmol.py --type f -exec cat -n {} \;

echo ""
echo "=== Checking ipsuite/pack directory structure ==="
find ipsuite/pack -type f -name "*.py" | head -20

echo ""
echo "=== Contents of ipsuite/pack/__init__.py ==="
if [ -f "ipsuite/pack/__init__.py" ]; then
  cat -n ipsuite/pack/__init__.py
else
  echo "File not found"
fi

echo ""
echo "=== Check if molify.pack exists ==="
fd "pack" ipsuite --type f --name "*.py" | rg molify

echo ""
echo "=== Checking test_pack.py to see exact imports and line context ==="
fd test_pack.py --type f -exec cat -n {} \;

Repository: zincware/IPSuite

Length of output: 1158

🏁 Script executed:

#!/bin/bash

echo "=== Contents of packmol.py ==="
cat ipsuite/configuration_generation/packmol.py

echo ""
echo "=== Directory structure under ipsuite ==="
ls -la ipsuite/

echo ""
echo "=== Looking for test_pack.py ==="
find . -name "test_pack.py" -type f

echo ""
echo "=== Contents of test_pack.py ==="
find . -name "test_pack.py" -type f -exec cat {} \;

echo ""
echo "=== Checking imports in packmol.py for 'pack' ==="
grep -n "import\|from" ipsuite/configuration_generation/packmol.py | head -20

Repository: zincware/IPSuite

Length of output: 9440

Correct the patch targets to reference the module where pack and znh5md.IO are actually imported.

The production code in ipsuite/configuration_generation/packmol.py imports pack via from molify.pack import pack (line 15). The test patches target ipsuite.pack.pack and ipsuite.pack.znh5md.IO, but ipsuite.pack is not an actual module in the codebase. Mocks must patch where the names are looked up, not where they're defined. Update the patches to:

`@patch`("ipsuite.configuration_generation.packmol.pack")
`@patch`("ipsuite.configuration_generation.packmol.znh5md")
def test_packmol_run_calls_pack(mock_znh5md, mock_pack):
    mock_pack.return_value = dummy_atoms()
    mock_znh5md.IO.return_value = MagicMock()
    # ... rest of test
🤖 Prompt for AI Agents 
In `@test_pack.py` around lines 73 - 90, The test patches target the wrong module;
update the patch decorators in test_packmol_run_calls_pack to patch the names
where Packmol looks them up (pack and znh5md.IO) in
ipsuite.configuration_generation.packmol rather than ipsuite.pack; specifically
change the patch targets to "ipsuite.configuration_generation.packmol.pack" and
"ipsuite.configuration_generation.packmol.znh5md" (and adapt the test signature
to mock_znh5md, mock_pack), then set mock_pack.return_value = dummy_atoms() and
mock_znh5md.IO.return_value = MagicMock() so Packmol.run() uses the injected
mocks.



# -----------------------------
# MultiPackmol inheritance test
# -----------------------------


@patch("ipsuite.pack.pack")
@patch("ipsuite.pack.znh5md.IO")
def test_multipackmol_multiple_configs(mock_io, mock_pack):
mock_pack.return_value = dummy_atoms()
mock_io.return_value = MagicMock()

node = MultiPackmol(
data=[[dummy_atoms()]],
count=[1],
density=1000,
n_configurations=3,
ratio=[1.0, 1.0, 2.0],
)

node.run()

assert mock_pack.call_count == 3
for call in mock_pack.call_args_list:
assert call.kwargs["ratio"] == [1.0, 1.0, 2.0]
Loading