pair_criteria: require cut_off for EnergyCriterion (bug-sweep #29)#5362
Draft
RudolfWeeber wants to merge 1 commit into
Draft
pair_criteria: require cut_off for EnergyCriterion (bug-sweep #29)#5362RudolfWeeber wants to merge 1 commit into
RudolfWeeber wants to merge 1 commit into
Conversation
EnergyCriterion()'s core constructor takes a System argument, so the core object is direct-initialized via make_shared<EnergyCriterion>(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<double>(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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 noreply@anthropic.com
🤖 Generated with Claude Code