core: fix TabulatedPotential min==max yielding invstepsize=NaN (bug-sweep #44)#5369
Draft
RudolfWeeber wants to merge 1 commit into
Draft
core: fix TabulatedPotential min==max yielding invstepsize=NaN (bug-sweep #44)#5369RudolfWeeber wants to merge 1 commit into
RudolfWeeber wants to merge 1 commit into
Conversation
…weep #44) A single-point tabulated potential (minval == maxval, force.size() == 1) is an intentionally supported, constant potential: the constructor only throws when the force table does not hold exactly one element in this case (mirrored by testsuite/python/tabulated.py). However the constructor then computed invstepsize = (size - 1) / (maxval - minval) = 0/0 = NaN, which poisoned every force()/energy() evaluation and triggered an out-of-bounds read of the size-1 table in Utils::linear_interpolation (static_cast<int>(NaN) is undefined, table[uind + 1] reads element [1]). Fix: - TabulatedPotential.cpp: guard the invstepsize computation so the degenerate denominator yields invstepsize = 0 (mirroring the existing minval == -1. sentinel branch). With invstepsize = 0 the interpolation collapses to the single constant table value, which is physically correct for a one-point table. - linear_interpolation.hpp: harden the residual latent out-of-bounds read so the upper data point is never accessed past the end of a single-point table (the term is multiplied by dx == 0 anyway). Adds src/core/unit_tests/TabulatedPotential_test.cpp covering the degenerate single-point table and a regular multi-point table. 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.
A single-point tabulated potential (minval == maxval, force.size() == 1) is
an intentionally supported, constant potential: the constructor only throws
when the force table does not hold exactly one element in this case
(mirrored by testsuite/python/tabulated.py). However the constructor then
computed invstepsize = (size - 1) / (maxval - minval) = 0/0 = NaN, which
poisoned every force()/energy() evaluation and triggered an out-of-bounds
read of the size-1 table in Utils::linear_interpolation
(static_cast(NaN) is undefined, table[uind + 1] reads element [1]).
Fix:
degenerate denominator yields invstepsize = 0 (mirroring the existing
minval == -1. sentinel branch). With invstepsize = 0 the interpolation
collapses to the single constant table value, which is physically correct
for a one-point table.
the upper data point is never accessed past the end of a single-point table
(the term is multiplied by dx == 0 anyway).
Adds src/core/unit_tests/TabulatedPotential_test.cpp covering the degenerate
single-point table and a regular multi-point table.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
🤖 Generated with Claude Code