From 0bfc809ee8276a083979e8566141f14a9c8ab0fa Mon Sep 17 00:00:00 2001 From: Rudolf Weeber Date: Sat, 13 Jun 2026 17:11:11 +0200 Subject: [PATCH] pair_criteria: require cut_off for EnergyCriterion (bug-sweep #29) EnergyCriterion()'s core constructor takes a System argument, so the core object is direct-initialized via make_shared(system) with no value-initialization. The member m_cut_off had no in-class initializer and was omitted from the ctor init list, and neither the Python nor the script-interface layer enforced cut_off as a required parameter. Constructing EnergyCriterion() without cut_off was therefore accepted and left m_cut_off indeterminate; a subsequent get_params() or decide() read uninitialized memory (nondeterministic clustering result). Fix: add a do_construct override to the script-interface EnergyCriterion that reads cut_off via the two-arg get_value(params, "cut_off"), matching the established required-parameter convention (BindCenters, HollowConicalFrustum). Omitting cut_off now raises RuntimeError "Parameter 'cut_off' is missing." cut_off is THE parameter that defines the criterion, so prevention (required arg) is correct rather than silently defaulting to a meaningless 0.0. As defense in depth, also give the core member an in-class initializer (double m_cut_off = 0.0;) so the raw read is well-defined regardless of construction path. Regression test added to testsuite/python/pair_criteria.py. Co-Authored-By: Claude Opus 4.8 --- src/core/pair_criteria/EnergyCriterion.hpp | 6 +++++- .../pair_criteria/EnergyCriterion.hpp | 9 +++++++++ testsuite/python/pair_criteria.py | 14 ++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/core/pair_criteria/EnergyCriterion.hpp b/src/core/pair_criteria/EnergyCriterion.hpp index d0c607f450..3a8d249874 100644 --- a/src/core/pair_criteria/EnergyCriterion.hpp +++ b/src/core/pair_criteria/EnergyCriterion.hpp @@ -53,7 +53,11 @@ class EnergyCriterion : public PairCriterion { void set_cut_off(double c) { m_cut_off = c; } private: - double m_cut_off; + // in-class initializer: the user-provided ctor takes a System argument and + // thus direct-initializes the object (no value-initialization), so without + // this default m_cut_off would be indeterminate until set_cut_off runs + // (bug-sweep #29). + double m_cut_off = 0.0; System::System const &m_system; }; } // namespace PairCriteria diff --git a/src/script_interface/pair_criteria/EnergyCriterion.hpp b/src/script_interface/pair_criteria/EnergyCriterion.hpp index fe55fd1af1..9e9b78caf1 100644 --- a/src/script_interface/pair_criteria/EnergyCriterion.hpp +++ b/src/script_interface/pair_criteria/EnergyCriterion.hpp @@ -46,6 +46,15 @@ class EnergyCriterion : public PairCriterion { [this]() { return m_c->get_cut_off(); }}}); } + void do_construct(VariantMap const ¶ms) override { + // cut_off is the parameter that defines the criterion and has no + // meaningful default; enforce it as a required construction parameter. + // get_value throws "Parameter 'cut_off' is missing." when it is omitted, + // which prevents the core m_cut_off threshold from being left + // uninitialized (bug-sweep #29). + m_c->set_cut_off(get_value(params, "cut_off")); + } + std::shared_ptr<::PairCriteria::PairCriterion> pair_criterion() const override { return m_c; diff --git a/testsuite/python/pair_criteria.py b/testsuite/python/pair_criteria.py index 32c354e0ca..a25e7536ad 100644 --- a/testsuite/python/pair_criteria.py +++ b/testsuite/python/pair_criteria.py @@ -93,6 +93,20 @@ def test_energy_crit_non_periodic(self): self.assertTrue(not ec.decide(self.p1, self.p2)) self.assertTrue(not ec.decide(self.p1.id, self.p2.id)) + def test_energy_crit_requires_cut_off(self): + # Regression test for bug-sweep #29: constructing an EnergyCriterion + # without the mandatory ``cut_off`` parameter must be rejected. On the + # unfixed code the omitted ``cut_off`` was silently accepted, leaving + # the core member ``m_cut_off`` uninitialized (the core object is + # direct-initialized via make_shared(system), so it is + # not value-initialized), and a subsequent get_params()/decide() read + # indeterminate memory. ``cut_off`` is THE parameter that defines the + # criterion, so it must be required, like the ``distance`` of + # DistanceCriterion. + with self.assertRaisesRegex( + RuntimeError, "Parameter 'cut_off' is missing."): + espressomd.pair_criteria.EnergyCriterion() + def test_bond_crit(self): bt = 0 bc = espressomd.pair_criteria.BondCriterion(bond_type=bt)