From ab6f183ba962c9029df87e4bfdf69a99764a0d9c Mon Sep 17 00:00:00 2001 From: njzjz-bot Date: Mon, 24 Aug 2026 03:03:51 +0800 Subject: [PATCH 1/2] fix: keep partial relaxation groups Use ceiling division for CIF relaxation tasks, reject invalid group sizes, and cover remainder and undersized inputs. Closes #350 Coding-Agent: Codex Codex-Version: codex-cli 0.149.1 Model: gpt-5.6-sol Reasoning-Effort: xhigh --- dpgen2/op/prep_relax.py | 6 ++++- tests/op/test_prep_relax.py | 52 +++++++++++++++++++++++++++++-------- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/dpgen2/op/prep_relax.py b/dpgen2/op/prep_relax.py index 1ee2869a..40a25312 100644 --- a/dpgen2/op/prep_relax.py +++ b/dpgen2/op/prep_relax.py @@ -41,7 +41,11 @@ def execute( ncifs = len(ip["cifs"]) config = ip["expl_config"] group_size = config["relax_group_size"] - ntasks = int(ncifs / group_size) + if group_size <= 0: + raise ValueError("relax_group_size must be greater than zero") + # Ceiling division keeps a final partial group instead of silently + # dropping CIFs when their count is not divisible by group_size. + ntasks = (ncifs + group_size - 1) // group_size task_paths = [] for i in range(ntasks): task_dir = Path("task.%06d" % i) diff --git a/tests/op/test_prep_relax.py b/tests/op/test_prep_relax.py index 83dcc2b7..b44e77ae 100644 --- a/tests/op/test_prep_relax.py +++ b/tests/op/test_prep_relax.py @@ -15,32 +15,62 @@ class TestPrepRelax(unittest.TestCase): - def testPrepRelax(self): + def setUp(self): + self.cifs = [] + + def _make_cifs(self, count): cifs = [] - for i in range(4): + for i in range(count): p = Path("%i.cif" % i) p.write_text("Mocked cif.") cifs.append(p) + self.cifs.extend(cifs) + return cifs + + def _run_prep_relax(self, ncifs, group_size): op_in = OPIO( { "expl_config": { - "relax_group_size": 2, + "relax_group_size": group_size, }, - "cifs": cifs, + "cifs": self._make_cifs(ncifs), } ) op = PrepRelax() - op_out = op.execute(op_in) + return op.execute(op_in) + + def test_prep_relax(self): + op_out = self._run_prep_relax(4, 2) self.assertEqual(op_out["ntasks"], 2) self.assertEqual(len(op_out["task_paths"]), 2) for i, task_path in enumerate(op_out["task_paths"]): self.assertEqual(str(task_path), "task.%06d" % i) self.assertEqual(len(list(task_path.iterdir())), 2) + def test_keeps_partial_final_group(self): + """Assign every CIF when the final task is not a full group.""" + op_out = self._run_prep_relax(5, 2) + + self.assertEqual(op_out["ntasks"], 3) + self.assertEqual( + [len(list(task_path.iterdir())) for task_path in op_out["task_paths"]], + [2, 2, 1], + ) + + def test_creates_task_when_group_is_larger_than_input(self): + op_out = self._run_prep_relax(1, 2) + + self.assertEqual(op_out["ntasks"], 1) + self.assertEqual(len(list(op_out["task_paths"][0].iterdir())), 1) + + def test_rejects_non_positive_group_size(self): + with self.assertRaisesRegex(ValueError, "greater than zero"): + self._run_prep_relax(1, 0) + def tearDown(self): - for i in range(2): - if os.path.isdir("task.%06d" % i): - shutil.rmtree("task.%06d" % i) - for i in range(4): - if os.path.isfile("%s.cif" % i): - os.remove("%s.cif" % i) + for task_path in Path().glob("task.[0-9][0-9][0-9][0-9][0-9][0-9]"): + if task_path.is_dir(): + shutil.rmtree(task_path) + for cif in self.cifs: + if cif.is_file(): + os.remove(cif) From 0255a205d1a3efc4f4855ef23939f81604b00733 Mon Sep 17 00:00:00 2001 From: njzjz-bot Date: Sat, 29 Aug 2026 23:06:39 +0800 Subject: [PATCH 2/2] test: cover negative relaxation group sizes Exercise both zero and negative group sizes so the non-positive runtime guard cannot regress to an equality-only check. Coding-Agent: Codex Codex-Version: codex-cli 0.151.0 Model: gpt-5.6-sol Reasoning-Effort: xhigh --- tests/op/test_prep_relax.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/op/test_prep_relax.py b/tests/op/test_prep_relax.py index b44e77ae..b8bd60a1 100644 --- a/tests/op/test_prep_relax.py +++ b/tests/op/test_prep_relax.py @@ -64,8 +64,10 @@ def test_creates_task_when_group_is_larger_than_input(self): self.assertEqual(len(list(op_out["task_paths"][0].iterdir())), 1) def test_rejects_non_positive_group_size(self): - with self.assertRaisesRegex(ValueError, "greater than zero"): - self._run_prep_relax(1, 0) + for group_size in (0, -1): + with self.subTest(group_size=group_size): + with self.assertRaisesRegex(ValueError, "greater than zero"): + self._run_prep_relax(1, group_size) def tearDown(self): for task_path in Path().glob("task.[0-9][0-9][0-9][0-9][0-9][0-9]"):