From 338bd013652e8eee7689321641cd177ac56d36fd Mon Sep 17 00:00:00 2001 From: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Date: Sat, 26 Sep 2026 09:14:38 +0100 Subject: [PATCH] Accept moved vectors in table data loaders --- models/utilities/CMakeLists.txt | 1 + models/utilities/table_interp_cpp/README.md | 37 +++ .../include/generic_multi_input_table.hh | 4 + .../include/table_independent_variable.hh | 3 + .../src/generic_multi_input_table.cc | 36 ++- .../src/table_independent_variable.cc | 34 ++- .../test/table_move_data_test.cc | 222 ++++++++++++++++++ 7 files changed, 328 insertions(+), 9 deletions(-) create mode 100644 models/utilities/table_interp_cpp/test/table_move_data_test.cc diff --git a/models/utilities/CMakeLists.txt b/models/utilities/CMakeLists.txt index 788de31e..def742e7 100644 --- a/models/utilities/CMakeLists.txt +++ b/models/utilities/CMakeLists.txt @@ -111,4 +111,5 @@ add_cml_tests( cml_message/test/cml_message_test.cc double_to_words/test/convert_double_to_words_test.cc table_interp_cpp/test/table_independent_variable_test.cc + table_interp_cpp/test/table_move_data_test.cc ) diff --git a/models/utilities/table_interp_cpp/README.md b/models/utilities/table_interp_cpp/README.md index 34d66340..057d4c3f 100644 --- a/models/utilities/table_interp_cpp/README.md +++ b/models/utilities/table_interp_cpp/README.md @@ -34,3 +34,40 @@ This model is independently documented in the docs directory. ## Verification This model contains independent verification test cases in the verif directory. + +## Loading vectors without copying + +`TableIndependentVariable::load_data` and `GenericMultiInputTable::load_data` +accept rvalue `DoubleVec` (`std::vector`) arguments. Use `std::move` when +transferring a large data vector that the caller no longer needs: + +```cpp +#include + +double input = 0.5; +double output = 0.0; +TableIndependentVariable axis(input); +DoubleVec calibration{0.0, 1.0, 2.0}; +if (!axis.load_data(std::move(calibration)) || !axis.initialize()) { + return; +} + +GenericMultiInputTable table(output); +table.add_independent(axis); +DoubleVec samples{10.0, 20.0, 30.0}; +if (!table.load_data(std::move(samples), SizeVec{1, 3}) || !table.initialize()) { + return; +} +table.update(); // output is 15.0 +``` + +The same move overload is inherited by the single-input table classes. Existing +lvalue and pointer overloads continue to copy. Validation is shared between the +copy and move paths: independent data must be nonempty and monotonic, and +dependent data must match its dimensions and output count. Rejected vector +arguments are not moved from. After a successful move, the caller's vector is +valid but its contents are unspecified; reassign it before using its data again. + +These methods keep the existing reload rules. Independent-variable data must +be cleared before reloading; a dependent table can replace previously loaded +data and retains its existing warning. diff --git a/models/utilities/table_interp_cpp/include/generic_multi_input_table.hh b/models/utilities/table_interp_cpp/include/generic_multi_input_table.hh index 70f68047..1bfbc814 100644 --- a/models/utilities/table_interp_cpp/include/generic_multi_input_table.hh +++ b/models/utilities/table_interp_cpp/include/generic_multi_input_table.hh @@ -79,6 +79,9 @@ class GenericMultiInputTable const SizeVec &dim_list); bool load_data( const DoubleVec & data_in, const SizeVec &dim_list); + // Move validated vector storage into the table; rejected inputs are unchanged. + bool load_data( DoubleVec && data_in, + const SizeVec &dim_list); void add_dependent( double & new_dep_var); void append_dependent_data( double & new_dep_var, @@ -124,6 +127,7 @@ class GenericMultiInputTable bool load_data_internal_check( const SizeVec &dim_list ); bool copy_data(const double * data_in); bool copy_data(const DoubleVec & data_in); + bool check_vector_data(const DoubleVec & data_in); size_t configure_internal_data_structure(); void configure_support_arrays(); virtual void generate_base_values(); diff --git a/models/utilities/table_interp_cpp/include/table_independent_variable.hh b/models/utilities/table_interp_cpp/include/table_independent_variable.hh index 54b3cbd0..e1aad801 100644 --- a/models/utilities/table_interp_cpp/include/table_independent_variable.hh +++ b/models/utilities/table_interp_cpp/include/table_independent_variable.hh @@ -183,6 +183,8 @@ public: virtual bool load_data( const double* const data_in, size_t size_in); virtual bool load_data( const DoubleVec & data_in); + // Move validated vector storage into the table; rejected inputs are unchanged. + virtual bool load_data( DoubleVec && data_in); bool initialize(); @@ -225,6 +227,7 @@ private: void tag_as_off_table_back(); void tag_as_off_table_front(); void generate_fraction(); + bool check_data( const DoubleVec & data_in); bool check_monotonicity( const std::vector & data_in); }; diff --git a/models/utilities/table_interp_cpp/src/generic_multi_input_table.cc b/models/utilities/table_interp_cpp/src/generic_multi_input_table.cc index 32aa4f0c..f548a61f 100644 --- a/models/utilities/table_interp_cpp/src/generic_multi_input_table.cc +++ b/models/utilities/table_interp_cpp/src/generic_multi_input_table.cc @@ -13,6 +13,7 @@ LIBRARY DEPENDENCIES: #include #include +#include #include "../include/generic_multi_input_table.hh" #include "../include/table_independent_variable.hh" @@ -103,10 +104,23 @@ GenericMultiInputTable::load_data( return copy_data(data_in); } +/****************************************************************************/ +bool +GenericMultiInputTable::load_data( + DoubleVec && data_in, + const SizeVec &dim_list) +{ + if (!load_data_internal_check(dim_list) || !check_vector_data(data_in)) { + return false; + } + data = std::move(data_in); + data_loaded = true; + return true; +} + /***************************************************************************** load_data_internal_check -Purpose:(Perform internal checks on data, common to both methods of loading - data) +Purpose:(Perform internal checks common to all data-loading methods.) *****************************************************************************/ bool GenericMultiInputTable::load_data_internal_check( @@ -679,6 +693,22 @@ GenericMultiInputTable::copy_data( bool GenericMultiInputTable::copy_data( const DoubleVec & data_in) +{ + if (!check_vector_data(data_in)) { + return false; + } + data = data_in; + data_loaded = true; + return true; +} + +/***************************************************************************** +check_vector_data +Purpose:(Check vector dimensions before copying or moving the data.) +*****************************************************************************/ +bool +GenericMultiInputTable::check_vector_data( + const DoubleVec & data_in) { // Configure internal data structure, abort on error const size_t total_data_elements = configure_internal_data_structure(); @@ -703,8 +733,6 @@ GenericMultiInputTable::copy_data( num_data_elements_per_increment_of_index.clear(); return false; } - data = data_in; - data_loaded = true; return true; } diff --git a/models/utilities/table_interp_cpp/src/table_independent_variable.cc b/models/utilities/table_interp_cpp/src/table_independent_variable.cc index 2992fde4..7dbfcb7a 100644 --- a/models/utilities/table_interp_cpp/src/table_independent_variable.cc +++ b/models/utilities/table_interp_cpp/src/table_independent_variable.cc @@ -18,6 +18,7 @@ LIBRARY DEPENDENCIES: #include #include #include +#include #include #include "cml/models/utilities/cml_message/include/cml_message.hh" #include "cml/models/utilities/math_utils/include/math_utils.hh" @@ -104,6 +105,34 @@ TableIndependentVariable::load_data( bool TableIndependentVariable::load_data( const DoubleVec & data_in) +{ + if (!check_data(data_in)) { + return false; + } + data = data_in; + data_loaded = true; + return true; +} +/****************************************************************************/ +bool +TableIndependentVariable::load_data( + DoubleVec && data_in) +{ + if (!check_data(data_in)) { + return false; + } + data = std::move(data_in); + data_loaded = true; + return true; +} + +/***************************************************************************** +check_data +Purpose:(Validate input before copying or moving its storage.) +*****************************************************************************/ +bool +TableIndependentVariable::check_data( + const DoubleVec & data_in) { if (data_loaded) { CMLMessage::error( @@ -136,11 +165,6 @@ TableIndependentVariable::load_data( "There is nothing wrong, just nothing to look up; output value " "is constant.\n"); } - // copy the data. - data = data_in; - - data_loaded = true; - return true; } diff --git a/models/utilities/table_interp_cpp/test/table_move_data_test.cc b/models/utilities/table_interp_cpp/test/table_move_data_test.cc new file mode 100644 index 00000000..d44e26fe --- /dev/null +++ b/models/utilities/table_interp_cpp/test/table_move_data_test.cc @@ -0,0 +1,222 @@ +#include "../include/generic_multi_input_table.hh" +#include "../include/generic_single_input_table.hh" +#include "../include/table_independent_variable.hh" +#include "mocks/cml/cml_message_mock.hh" + +#include +#include +#include +#include +#include + +namespace { + +// Inspect protected storage without changing the model's public interface. +class InspectableTable : public GenericMultiInputTable { +public: + using GenericMultiInputTable::GenericMultiInputTable; + const DoubleVec& stored_data() const { return data; } +}; + +TEST(TableMoveData, IndependentTakesVectorStorage) { + double input = 512.5; + TableIndependentVariable table(input); + DoubleVec values(1024); + std::iota(values.begin(), values.end(), 0.0); + const double* const storage = values.data(); + + ASSERT_TRUE(table.load_data(std::move(values))); + EXPECT_EQ(table.data.data(), storage); + EXPECT_EQ(table.get_size(), 1024U); + ASSERT_TRUE(table.initialize()); + EXPECT_EQ(table.get_index(), 512U); + EXPECT_DOUBLE_EQ(table.fraction, 0.5); +} + +TEST(TableMoveData, IndependentAcceptsDecreasingAndSinglePointData) { + double input = 1.5; + TableIndependentVariable table(input); + DoubleVec decreasing{3.0, 2.0, 1.0}; + const double* const storage = decreasing.data(); + ASSERT_TRUE(table.load_data(std::move(decreasing))); + EXPECT_EQ(table.data.data(), storage); + EXPECT_FALSE(table.is_table_increasing()); + ASSERT_TRUE(table.initialize()); + EXPECT_EQ(table.get_index(), 1U); + EXPECT_DOUBLE_EQ(table.fraction, 0.5); + + TableIndependentVariable constant(input); + DoubleVec one_point{2.0}; + const double* const one_storage = one_point.data(); + CMLMessage::Mock messages; + EXPECT_CALL(messages, publish(CMLMessage::Warning, testing::_, testing::_, testing::_)); + ASSERT_TRUE(constant.load_data(std::move(one_point))); + EXPECT_EQ(constant.data.data(), one_storage); + ASSERT_TRUE(constant.initialize()); + EXPECT_EQ(constant.get_index(), 0U); +} + +TEST(TableMoveData, IndependentRejectsWithoutConsumingInput) { + const std::vector invalid{{}, {1.0, 1.0}, {0.0, 2.0, 1.0}}; + for (const auto& expected : invalid) { + double input = 0.0; + TableIndependentVariable table(input); + DoubleVec values = expected; + const double* const storage = values.data(); + CMLMessage::Mock messages; + EXPECT_CALL(messages, publish(CMLMessage::Error, testing::_, testing::_, testing::_)); + EXPECT_FALSE(table.load_data(std::move(values))); + EXPECT_EQ(values, expected); + EXPECT_EQ(values.data(), storage); + EXPECT_FALSE(table.is_data_loaded()); + } +} + +TEST(TableMoveData, IndependentRejectsReloadUntilCleared) { + double input = 0.5; + TableIndependentVariable table(input); + const DoubleVec original{0.0, 1.0}; + ASSERT_TRUE(table.load_data(original)); + DoubleVec replacement{0.0, 2.0, 4.0}; + const double* const storage = replacement.data(); + { + CMLMessage::Mock messages; + EXPECT_CALL(messages, publish(CMLMessage::Error, testing::_, testing::_, testing::_)); + EXPECT_FALSE(table.load_data(std::move(replacement))); + } + EXPECT_EQ(table.data, original); + EXPECT_EQ(replacement.data(), storage); + EXPECT_EQ(replacement.size(), 3U); + table.clear_data(); + ASSERT_TRUE(table.load_data(std::move(replacement))); + EXPECT_EQ(table.data.data(), storage); +} + +TEST(TableMoveData, IndependentLvalueStillCopies) { + double input = 0.0; + TableIndependentVariable table(input); + const DoubleVec values{0.0, 1.0, 2.0}; + ASSERT_TRUE(table.load_data(values)); + EXPECT_EQ(table.data, values); + EXPECT_NE(table.data.data(), values.data()); +} + +TEST(TableMoveData, PointerOverloadsStillCopy) { + double input = 0.5; + double output = 0.0; + double values[]{0.0, 1.0, 2.0}; + TableIndependentVariable independent(input); + InspectableTable dependent(output); + ASSERT_TRUE(independent.load_data(values, 3)); + ASSERT_TRUE(dependent.load_data(values, SizeVec{1, 3})); + EXPECT_NE(independent.data.data(), values); + EXPECT_NE(dependent.stored_data().data(), values); + values[0] = 100.0; + EXPECT_DOUBLE_EQ(independent.data.front(), 0.0); + EXPECT_DOUBLE_EQ(dependent.stored_data().front(), 0.0); +} + +TEST(TableMoveData, DependentTakesVectorStorageAndInterpolates) { + double input = 0.5; + double output = 0.0; + TableIndependentVariable independent(input); + ASSERT_TRUE(independent.load_data(DoubleVec{0.0, 1.0, 2.0})); + ASSERT_TRUE(independent.initialize()); + InspectableTable table(output); + table.add_independent(independent); + DoubleVec values{10.0, 20.0, 30.0}; + const double* const storage = values.data(); + + ASSERT_TRUE(table.load_data(std::move(values), SizeVec{1, 3})); + EXPECT_EQ(table.stored_data().data(), storage); + ASSERT_TRUE(table.initialize()); + ASSERT_TRUE(table.update()); + EXPECT_DOUBLE_EQ(output, 15.0); + input = 1.5; + ASSERT_TRUE(independent.update()); + ASSERT_TRUE(table.update()); + EXPECT_DOUBLE_EQ(output, 25.0); +} + +TEST(TableMoveData, DependentSupportsMultipleDimensions) { + double x = 0.25; + double y = 0.5; + double output = 0.0; + TableIndependentVariable first(x); + TableIndependentVariable second(y); + ASSERT_TRUE(first.load_data(DoubleVec{0.0, 1.0})); + ASSERT_TRUE(second.load_data(DoubleVec{0.0, 1.0})); + ASSERT_TRUE(first.initialize()); + ASSERT_TRUE(second.initialize()); + InspectableTable table(output); + table.add_independent(first); + table.add_independent(second); + DoubleVec values{0.0, 2.0, 4.0, 6.0}; + const double* const storage = values.data(); + + ASSERT_TRUE(table.load_data(std::move(values), SizeVec{1, 2, 2})); + EXPECT_EQ(table.stored_data().data(), storage); + ASSERT_TRUE(table.initialize()); + ASSERT_TRUE(table.update()); + EXPECT_DOUBLE_EQ(output, 2.0); // 4*x + 2*y +} + +TEST(TableMoveData, DependentRejectsWithoutConsumingInput) { + const std::vector dimensions{{}, {1}, {2, 3}, {1, 0}, {1, 4}}; + for (const auto& shape : dimensions) { + double output = 0.0; + InspectableTable table(output); + DoubleVec values{10.0, 20.0, 30.0}; + const DoubleVec expected = values; + const double* const storage = values.data(); + CMLMessage::Mock messages; + EXPECT_CALL(messages, publish(CMLMessage::Error, testing::_, testing::_, testing::_)) + .Times(testing::AtLeast(1)); + EXPECT_FALSE(table.load_data(std::move(values), shape)); + EXPECT_EQ(values, expected); + EXPECT_EQ(values.data(), storage); + EXPECT_FALSE(table.is_data_loaded()); + // The existing copy overload must reject the same dimensions and size. + InspectableTable copy_table(output); + EXPECT_FALSE(copy_table.load_data(expected, shape)); + EXPECT_FALSE(copy_table.is_data_loaded()); + } +} + +TEST(TableMoveData, DependentLvalueStillCopies) { + double output = 0.0; + InspectableTable table(output); + const DoubleVec values{10.0, 20.0}; + ASSERT_TRUE(table.load_data(values, SizeVec{1, 2})); + EXPECT_EQ(table.stored_data(), values); + EXPECT_NE(table.stored_data().data(), values.data()); +} + +TEST(TableMoveData, DependentReloadMovesReplacementStorage) { + double output = 0.0; + InspectableTable table(output); + ASSERT_TRUE(table.load_data(DoubleVec{10.0, 20.0}, SizeVec{1, 2})); + DoubleVec replacement{30.0, 40.0, 50.0}; + const double* const storage = replacement.data(); + CMLMessage::Mock messages; + EXPECT_CALL(messages, publish(CMLMessage::Warning, testing::_, testing::_, testing::_)); + ASSERT_TRUE(table.load_data(std::move(replacement), SizeVec{1, 3})); + EXPECT_EQ(table.stored_data().data(), storage); + EXPECT_EQ(table.get_data_size(), 3U); +} + +TEST(TableMoveData, SingleInputTableInheritsMoveLoading) { + double input = 0.5; + double output = 0.0; + TableIndependentVariable independent(input); + ASSERT_TRUE(independent.load_data(DoubleVec{0.0, 1.0})); + ASSERT_TRUE(independent.initialize()); + GenericSingleInputTable table(output); + table.add_independent(independent); + ASSERT_TRUE(table.load_data(DoubleVec{20.0, 40.0}, SizeVec{1, 2})); + ASSERT_TRUE(table.initialize()); + ASSERT_TRUE(table.update()); + EXPECT_DOUBLE_EQ(output, 30.0); +} + +} // namespace