Skip to content

Commit e3480cf

Browse files
committed
refactor(api_cc): state the charge-state domain with relational tests
Flooring leaves an integer where it is and moves everything else down, so a strict drop is exactly a fractional part, and asking the value to be inside its bounds says the same as asking it not to be outside them for every value a table can address. Both forms are exact, and neither is an equality between floating-point values, which CodeQL flags because it is usually an epsilon mistake. The range test also gains a NaN, which compares false against every bound and so now falls out of it rather than through it.
1 parent 52ce83b commit e3480cf

2 files changed

Lines changed: 18 additions & 2 deletions

File tree

source/api_cc/include/DeepPot.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,10 @@ class DeepPotBackend : public DeepBaseModelBackend {
220220
* mirror ``deepmd/utils/charge_state.py``, which holds the same contract
221221
* for the Python boundaries.
222222
*
223+
* Both tests are relational rather than equalities, which keeps them exact
224+
* and lets a NaN, which compares false against everything, fall out of the
225+
* range test rather than through it.
226+
*
223227
* @param[in] charge_spin The condition, as ``{charge, multiplicity}``.
224228
**/
225229
static void require_addressable_charge_spin(
@@ -230,14 +234,16 @@ class DeepPotBackend : public DeepBaseModelBackend {
230234
const std::size_t checked = std::min<std::size_t>(charge_spin.size(), 2);
231235
for (std::size_t ii = 0; ii < checked; ++ii) {
232236
const double value = charge_spin[ii];
233-
if (value != std::floor(value)) {
237+
// Flooring leaves an integer where it is and moves everything else
238+
// down, so a strict drop is exactly a fractional part.
239+
if (std::floor(value) < value) {
234240
throw deepmd::deepmd_exception(
235241
std::string("the ") + names[ii] +
236242
" must be an integer, which indexes one row of its embedding "
237243
"table, but is " +
238244
std::to_string(value));
239245
}
240-
if (value < lows[ii] || value >= highs[ii]) {
246+
if (!(value >= lows[ii] && value < highs[ii])) {
241247
throw deepmd::deepmd_exception(
242248
std::string("the ") + names[ii] + " must lie in [" +
243249
std::to_string(lows[ii]) + ", " + std::to_string(highs[ii]) +

source/api_cc/tests/test_deeppot_chg_spin_pt.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <algorithm>
1212
#include <cmath>
1313
#include <fstream>
14+
#include <limits>
1415
#include <vector>
1516

1617
#include "DeepPot.h"
@@ -125,6 +126,15 @@ TYPED_TEST(TestInferDeepPotChgSpinPt,
125126
EXPECT_THROW(dp.set_charge_spin({100.0, 1.0}), deepmd::deepmd_exception);
126127
EXPECT_THROW(dp.set_charge_spin({0.0, -1.0}), deepmd::deepmd_exception);
127128
EXPECT_THROW(dp.set_charge_spin({0.0, 100.0}), deepmd::deepmd_exception);
129+
130+
// A NaN compares false against every bound, so it is refused only because
131+
// the range test asks the value to be inside rather than outside.
132+
const double nan = std::numeric_limits<double>::quiet_NaN();
133+
const double inf = std::numeric_limits<double>::infinity();
134+
EXPECT_THROW(dp.set_charge_spin({nan, 1.0}), deepmd::deepmd_exception);
135+
EXPECT_THROW(dp.set_charge_spin({0.0, nan}), deepmd::deepmd_exception);
136+
EXPECT_THROW(dp.set_charge_spin({inf, 1.0}), deepmd::deepmd_exception);
137+
EXPECT_THROW(dp.set_charge_spin({-inf, 1.0}), deepmd::deepmd_exception);
128138
}
129139

130140
// An installed state has to reach the evaluations that follow it, and a

0 commit comments

Comments
 (0)