From c2cba71e29260d9a98b188dfcabaeda97cf3c44f Mon Sep 17 00:00:00 2001 From: "Lars T. Kyllingstad" Date: Mon, 25 Feb 2019 15:14:38 +0100 Subject: [PATCH 1/5] Synchronised variable transfer This fixes issue #181, "Only transfer variable values at common communication points". --- src/cpp/algorithm.cpp | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/cpp/algorithm.cpp b/src/cpp/algorithm.cpp index 632ab55fc..fab372c99 100644 --- a/src/cpp/algorithm.cpp +++ b/src/cpp/algorithm.cpp @@ -7,6 +7,7 @@ #include "cse/exception.hpp" #include +#include #include #include #include @@ -216,19 +217,23 @@ class fixed_step_algorithm::impl void transfer_variables(const std::vector& connections) { for (const auto& c : connections) { - switch (c.input.type) { - case variable_type::real: - transfer_real(c.output, c.input); - break; - case variable_type::integer: - transfer_integer(c.output, c.input); - break; - case variable_type::boolean: - transfer_boolean(c.output, c.input); - break; - case variable_type::string: - transfer_string(c.output, c.input); - break; + const auto odf = simulators_[c.output.simulator].decimationFactor; + const auto idf = simulators_[c.input.simulator].decimationFactor; + if (stepCounter_ % std::lcm(odf, idf) == 0) { + switch (c.input.type) { + case variable_type::real: + transfer_real(c.output, c.input); + break; + case variable_type::integer: + transfer_integer(c.output, c.input); + break; + case variable_type::boolean: + transfer_boolean(c.output, c.input); + break; + case variable_type::string: + transfer_string(c.output, c.input); + break; + } } } } From d3bb10cdcb8ab933fefe72f76e7a66c822f9e31a Mon Sep 17 00:00:00 2001 From: "Lars T. Kyllingstad" Date: Tue, 5 Mar 2019 06:06:32 +0100 Subject: [PATCH 2/5] Get rid of some magic numbers in test --- test/cpp/multi_fixed_step_algorithm_test.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/cpp/multi_fixed_step_algorithm_test.cpp b/test/cpp/multi_fixed_step_algorithm_test.cpp index 12d5d01df..51739e990 100644 --- a/test/cpp/multi_fixed_step_algorithm_test.cpp +++ b/test/cpp/multi_fixed_step_algorithm_test.cpp @@ -37,6 +37,7 @@ int main() const cse::variable_index realOutIndex = 0; const cse::variable_index realInIndex = 1; + const cse::variable_index integerOutIndex = 0; double v = 0.0; auto idx0 = execution.add_slave(cse::make_pseudo_async(std::make_unique([&v](double /*x*/) { return ++v; })), "slave 0"); @@ -47,8 +48,8 @@ int main() auto idx2 = execution.add_slave(cse::make_pseudo_async(std::make_unique(nullptr, [&i](int /*x*/) { return ++i + 1; })), "slave 2"); execution.connect_variables( - cse::variable_id {0, cse::variable_type::real, realOutIndex}, - cse::variable_id {1, cse::variable_type::real, realInIndex}); + cse::variable_id {idx0, cse::variable_type::real, realOutIndex}, + cse::variable_id {idx1, cse::variable_type::real, realInIndex}); algorithm->set_stepsize_decimation_factor(idx0, 1); algorithm->set_stepsize_decimation_factor(idx1, 2); @@ -56,9 +57,9 @@ int main() auto observer2 = std::make_shared(); execution.add_observer(observer2); - observer2->start_observing(cse::variable_id {0, cse::variable_type::real, realOutIndex}); - observer2->start_observing(cse::variable_id {1, cse::variable_type::real, realOutIndex}); - observer2->start_observing(cse::variable_id {2, cse::variable_type::integer, 0}); + observer2->start_observing(cse::variable_id {idx0, cse::variable_type::real, realOutIndex}); + observer2->start_observing(cse::variable_id {idx1, cse::variable_type::real, realOutIndex}); + observer2->start_observing(cse::variable_id {idx2, cse::variable_type::integer, integerOutIndex}); // Run simulation auto simResult = execution.simulate_until(endTime); @@ -68,12 +69,12 @@ int main() double realValues0[numSamples]; cse::step_number steps0[numSamples]; cse::time_point timeValues0[numSamples]; - observer2->get_real_samples(0, realOutIndex, 1, gsl::make_span(realValues0, numSamples), gsl::make_span(steps0, numSamples), gsl::make_span(timeValues0, numSamples)); + observer2->get_real_samples(idx0, realOutIndex, 1, gsl::make_span(realValues0, numSamples), gsl::make_span(steps0, numSamples), gsl::make_span(timeValues0, numSamples)); double realValues1[numSamples]; cse::step_number steps1[numSamples]; cse::time_point timeValues1[numSamples]; - observer2->get_real_samples(1, realOutIndex, 1, gsl::make_span(realValues1, numSamples), gsl::make_span(steps1, numSamples), gsl::make_span(timeValues1, numSamples)); + observer2->get_real_samples(idx1, realOutIndex, 1, gsl::make_span(realValues1, numSamples), gsl::make_span(steps1, numSamples), gsl::make_span(timeValues1, numSamples)); double expectedReals0[] = {1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0}; double expectedReals1[] = {1.0, 2.0, 4.0, 6.0, 8.0}; @@ -81,7 +82,7 @@ int main() int intValues[numSamples]; cse::step_number steps[numSamples]; cse::time_point timeValues[numSamples]; - observer2->get_integer_samples(2, 0, 1, gsl::make_span(intValues, numSamples), gsl::make_span(steps, numSamples), gsl::make_span(timeValues, numSamples)); + observer2->get_integer_samples(idx2, integerOutIndex, 1, gsl::make_span(intValues, numSamples), gsl::make_span(steps, numSamples), gsl::make_span(timeValues, numSamples)); // int expectedInts[numSamples] = {0, 0, 2, 2, 2, 3, 3, 3, 4, 4}; // this is what we actually expect int expectedInts[] = {2, 3, 4}; From 4bb42adebc2b090e37536b1e162c994c838f08eb Mon Sep 17 00:00:00 2001 From: "Lars T. Kyllingstad" Date: Sun, 10 Mar 2019 22:51:41 +0100 Subject: [PATCH 3/5] Cache values only when the variables are set This ensures that only the variables that have actually been set() will be cached in `slave_simulator` and passed to `async_slave::set_variables()`. Previously, all *exposed* variables would always be in the cache. --- src/cpp/slave_simulator.cpp | 176 ++++++++++++++++++++++++++++-------- 1 file changed, 139 insertions(+), 37 deletions(-) diff --git a/src/cpp/slave_simulator.cpp b/src/cpp/slave_simulator.cpp index 412e0ddcb..37de8b148 100644 --- a/src/cpp/slave_simulator.cpp +++ b/src/cpp/slave_simulator.cpp @@ -21,14 +21,16 @@ struct var_view_type { using type = T; }; + template<> struct var_view_type { using type = std::string_view; }; + template -struct exposed_vars +struct get_variable_cache { std::vector indexes; boost::container::vector originalValues; @@ -59,36 +61,132 @@ struct exposed_vars } } - void set(variable_index i, typename var_view_type::type v) + void set_manipulator(variable_index i, std::function m) { - const auto it = indexMapping.find(i); - if (it != indexMapping.end()) { - originalValues[it->second] = v; - } else { + manipulators[indexMapping[i]] = m; + } + + void run_manipulators() + { + for (std::size_t i = 0; i < originalValues.size(); ++i) { + if (manipulators[i]) { + manipulatedValues[i] = manipulators[i](originalValues[i]); + } else { + manipulatedValues[i] = originalValues[i]; + } + } + } +}; + + +template +class set_variable_cache +{ +public: + void expose(variable_index i) + { + // TODO: Here, exposed_variable::lastValue will be initialised with + // the value T(). We ought to use the start value from the model + // description instead. + exposedVariables_.emplace(i, exposed_variable()); + } + + void set_value(variable_index i, typename var_view_type::type v) + { + assert(!hasRunManipulators_); + const auto it = exposedVariables_.find(i); + if (it == exposedVariables_.end()) { std::ostringstream oss; oss << "variable_index " << i << " not found in exposed variables. Variables must be exposed before calling set()"; throw std::out_of_range(oss.str()); } + it->second.lastValue = v; + if (it->second.arrayIndex < 0) { + it->second.arrayIndex = indexes_.size(); + assert(indexes_.size() == values_.size()); + indexes_.emplace_back(i); + values_.emplace_back(v); + } else { + assert(indexes_[it->second.arrayIndex] == i); + values_[it->second.arrayIndex] = v; + } } void set_manipulator(variable_index i, std::function m) { - manipulators[indexMapping[i]] = m; + assert(!hasRunManipulators_); + const auto it = exposedVariables_.find(i); + if (it == exposedVariables_.end()) { + std::ostringstream oss; + oss << "variable_index " << i + << " not found in exposed variables. Variables must be exposed before calling set_manipulator()"; + throw std::out_of_range(oss.str()); + } + if (it->second.arrayIndex < 0) { + // Ensure that the simulator receives an updated value. + it->second.arrayIndex = indexes_.size(); + assert(indexes_.size() == values_.size()); + indexes_.emplace_back(i); + values_.emplace_back(); + } + if (m) { + manipulators_[i] = m; + } else { + manipulators_.erase(i); + } } - void run_manipulators() + std::pair, gsl::span> manipulate_and_get() { - for (std::size_t i = 0; i < originalValues.size(); ++i) { - if (manipulators[i]) { - manipulatedValues[i] = manipulators[i](originalValues[i]); - } else { - manipulatedValues[i] = originalValues[i]; + if (!hasRunManipulators_) { + assert(indexes_.size() == values_.size()); + for (std::size_t i = 0; i < indexes_.size(); ++i) { + const auto iterator = manipulators_.find(indexes_[i]); + if (iterator != manipulators_.end()) { + values_[i] = iterator->second(values_[i]); + } } + hasRunManipulators_ = true; } + return std::pair(gsl::make_span(indexes_), gsl::make_span(values_)); } + + void reset() + { + for (auto index : indexes_) { + exposedVariables_.at(index).arrayIndex = -1; + } + indexes_.clear(); + values_.clear(); + hasRunManipulators_ = false; + } + +private: + struct exposed_variable + { + // The last set value of the variable. + T lastValue = T(); + + // The variable's index in the `indexes_` and `values_` arrays, or + // -1 if it hasn't been added to them yet. + std::ptrdiff_t arrayIndex = -1; + }; + + std::unordered_map exposedVariables_; + + // The manipulators associated with certain variables, and a flag that + // specifies whether they have been run on the values currently in + // `values_`. + std::unordered_map> manipulators_; + bool hasRunManipulators_ = false; + + // The indexes and values of the variables that will be set next. + std::vector indexes_; + boost::container::vector values_; }; + // Copies the contents of a contiguous-storage container or span into another. template void copy_contents(Src&& src, Tgt&& tgt) @@ -190,22 +288,22 @@ class slave_simulator::impl void set_real(variable_index index, double value) { - realSetCache_.set(index, value); + realSetCache_.set_value(index, value); } void set_integer(variable_index index, int value) { - integerSetCache_.set(index, value); + integerSetCache_.set_value(index, value); } void set_boolean(variable_index index, bool value) { - booleanSetCache_.set(index, value); + booleanSetCache_.set_value(index, value); } void set_string(variable_index index, std::string_view value) { - stringSetCache_.set(index, value); + stringSetCache_.set_value(index, value); } void set_real_input_manipulator( @@ -302,20 +400,24 @@ class slave_simulator::impl private: void set_variables() { - realSetCache_.run_manipulators(); - integerSetCache_.run_manipulators(); - booleanSetCache_.run_manipulators(); - stringSetCache_.run_manipulators(); + const auto [realIndexes, realValues] = realSetCache_.manipulate_and_get(); + const auto [integerIndexes, integerValues] = integerSetCache_.manipulate_and_get(); + const auto [booleanIndexes, booleanValues] = booleanSetCache_.manipulate_and_get(); + const auto [stringIndexes, stringValues] = stringSetCache_.manipulate_and_get(); slave_->set_variables( - gsl::make_span(realSetCache_.indexes), - gsl::make_span(realSetCache_.manipulatedValues), - gsl::make_span(integerSetCache_.indexes), - gsl::make_span(integerSetCache_.manipulatedValues), - gsl::make_span(booleanSetCache_.indexes), - gsl::make_span(booleanSetCache_.manipulatedValues), - gsl::make_span(stringSetCache_.indexes), - gsl::make_span(stringSetCache_.manipulatedValues)) + gsl::make_span(realIndexes), + gsl::make_span(realValues), + gsl::make_span(integerIndexes), + gsl::make_span(integerValues), + gsl::make_span(booleanIndexes), + gsl::make_span(booleanValues), + gsl::make_span(stringIndexes), + gsl::make_span(stringValues)) .get(); + realSetCache_.reset(); + integerSetCache_.reset(); + booleanSetCache_.reset(); + stringSetCache_.reset(); } void get_variables() @@ -341,15 +443,15 @@ class slave_simulator::impl std::string name_; cse::model_description modelDescription_; - exposed_vars realGetCache_; - exposed_vars integerGetCache_; - exposed_vars booleanGetCache_; - exposed_vars stringGetCache_; + get_variable_cache realGetCache_; + get_variable_cache integerGetCache_; + get_variable_cache booleanGetCache_; + get_variable_cache stringGetCache_; - exposed_vars realSetCache_; - exposed_vars integerSetCache_; - exposed_vars booleanSetCache_; - exposed_vars stringSetCache_; + set_variable_cache realSetCache_; + set_variable_cache integerSetCache_; + set_variable_cache booleanSetCache_; + set_variable_cache stringSetCache_; }; From 9c78d76eb65b3b7c1370231d3c14c55670d8cae1 Mon Sep 17 00:00:00 2001 From: "Lars T. Kyllingstad" Date: Sun, 10 Mar 2019 22:56:31 +0100 Subject: [PATCH 4/5] Test that values are transferred at correct times This test verifies that issue #181 has been fixed, i.e., that variable values are only transfered between simulators at common synchronisation points. --- test/cpp/multi_fixed_step_algorithm_test.cpp | 74 ++++++++++++++++++-- 1 file changed, 70 insertions(+), 4 deletions(-) diff --git a/test/cpp/multi_fixed_step_algorithm_test.cpp b/test/cpp/multi_fixed_step_algorithm_test.cpp index 51739e990..53c0fe5a0 100644 --- a/test/cpp/multi_fixed_step_algorithm_test.cpp +++ b/test/cpp/multi_fixed_step_algorithm_test.cpp @@ -7,9 +7,11 @@ #include #include +#include #include #include #include +#include #include @@ -17,6 +19,47 @@ #define REQUIRE(test) \ if (!(test)) throw std::runtime_error("Requirement not satisfied: " #test) + +// A slave that logs the time points when its set_real_variables() and +// set_integer_variables() functions get called. +class set_logging_mock_slave : public mock_slave +{ +public: + using mock_slave::mock_slave; + + cse::step_result do_step(cse::time_point currentT, cse::duration deltaT) override + { + time_ = currentT + deltaT; + return mock_slave::do_step(currentT, deltaT); + } + + void set_real_variables( + gsl::span variables, + gsl::span values) override + { + if (!variables.empty()) setRealLog_.insert(time_); + mock_slave::set_real_variables(variables, values); + } + + void set_integer_variables( + gsl::span variables, + gsl::span values) override + { + if (!variables.empty()) setIntegerLog_.insert(time_); + mock_slave::set_integer_variables(variables, values); + } + + const std::set& get_set_real_log() { return setRealLog_; } + + const std::set& get_set_integer_log() { return setIntegerLog_; } + +private: + cse::time_point time_; + std::set setRealLog_; + std::set setIntegerLog_; +}; + + int main() { try { @@ -38,18 +81,25 @@ int main() const cse::variable_index realOutIndex = 0; const cse::variable_index realInIndex = 1; const cse::variable_index integerOutIndex = 0; + const cse::variable_index integerInIndex = 1; double v = 0.0; - auto idx0 = execution.add_slave(cse::make_pseudo_async(std::make_unique([&v](double /*x*/) { return ++v; })), "slave 0"); + auto slave0 = std::make_shared([&v](double /*x*/) { return ++v; }); + auto idx0 = execution.add_slave(cse::make_pseudo_async(slave0), "slave 0"); - auto idx1 = execution.add_slave(cse::make_pseudo_async(std::make_unique()), "slave 1"); + auto slave1 = std::make_shared(); + auto idx1 = execution.add_slave(cse::make_pseudo_async(slave1), "slave 1"); int i = 0; - auto idx2 = execution.add_slave(cse::make_pseudo_async(std::make_unique(nullptr, [&i](int /*x*/) { return ++i + 1; })), "slave 2"); + auto slave2 = std::make_shared(nullptr, [&i](int /*x*/) { return ++i + 1; }); + auto idx2 = execution.add_slave(cse::make_pseudo_async(slave2), "slave 2"); execution.connect_variables( cse::variable_id {idx0, cse::variable_type::real, realOutIndex}, cse::variable_id {idx1, cse::variable_type::real, realInIndex}); + execution.connect_variables( + cse::variable_id {idx1, cse::variable_type::integer, integerOutIndex}, + cse::variable_id {idx2, cse::variable_type::integer, integerInIndex}); algorithm->set_stepsize_decimation_factor(idx0, 1); algorithm->set_stepsize_decimation_factor(idx1, 2); @@ -85,7 +135,7 @@ int main() observer2->get_integer_samples(idx2, integerOutIndex, 1, gsl::make_span(intValues, numSamples), gsl::make_span(steps, numSamples), gsl::make_span(timeValues, numSamples)); // int expectedInts[numSamples] = {0, 0, 2, 2, 2, 3, 3, 3, 4, 4}; // this is what we actually expect - int expectedInts[] = {2, 3, 4}; + int expectedInts[] = {2, 3, 4}; for (int k = 0; k < 10; k++) { REQUIRE(std::fabs(expectedReals0[k] - realValues0[k]) < 1e-9); @@ -97,6 +147,22 @@ int main() REQUIRE(expectedInts[k] == intValues[k]); } + const cse::time_point expectedSetRealTimes1[] = { + startTime, + startTime + 2 * stepSize, + startTime + 4 * stepSize, + startTime + 6 * stepSize, + startTime + 8 * stepSize}; + const auto& setRealLog1 = slave1->get_set_real_log(); + for (auto t: setRealLog1) std::cout << t.time_since_epoch().count() / 1000000.0 << std::endl; + REQUIRE(setRealLog1.size() == 5); + REQUIRE(std::equal(setRealLog1.begin(), setRealLog1.end(), expectedSetRealTimes1)); + + const cse::time_point expectedSetIntTimes2[] = {startTime, startTime + 6 * stepSize}; + const auto& setIntLog2 = slave2->get_set_integer_log(); + REQUIRE(setIntLog2.size() == 2); + REQUIRE(std::equal(setIntLog2.begin(), setIntLog2.end(), expectedSetIntTimes2)); + } catch (const std::exception& e) { std::cerr << "Error: " << e.what() << std::endl; return 1; From 4fe4fb5c58848f58a452acf194eb9ae9f3a9b2b2 Mon Sep 17 00:00:00 2001 From: "Lars T. Kyllingstad" Date: Tue, 12 Mar 2019 06:14:54 +0100 Subject: [PATCH 5/5] Add test for slow-to-fast-slave connections --- test/cpp/multi_fixed_step_algorithm_test.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/cpp/multi_fixed_step_algorithm_test.cpp b/test/cpp/multi_fixed_step_algorithm_test.cpp index 53c0fe5a0..fac670b83 100644 --- a/test/cpp/multi_fixed_step_algorithm_test.cpp +++ b/test/cpp/multi_fixed_step_algorithm_test.cpp @@ -100,6 +100,9 @@ int main() execution.connect_variables( cse::variable_id {idx1, cse::variable_type::integer, integerOutIndex}, cse::variable_id {idx2, cse::variable_type::integer, integerInIndex}); + execution.connect_variables( + cse::variable_id {idx2, cse::variable_type::integer, integerOutIndex}, + cse::variable_id {idx1, cse::variable_type::integer, integerInIndex}); algorithm->set_stepsize_decimation_factor(idx0, 1); algorithm->set_stepsize_decimation_factor(idx1, 2); @@ -158,6 +161,11 @@ int main() REQUIRE(setRealLog1.size() == 5); REQUIRE(std::equal(setRealLog1.begin(), setRealLog1.end(), expectedSetRealTimes1)); + const cse::time_point expectedSetIntTimes1[] = {startTime, startTime + 6 * stepSize}; + const auto& setIntLog1 = slave2->get_set_integer_log(); + REQUIRE(setIntLog1.size() == 2); + REQUIRE(std::equal(setIntLog1.begin(), setIntLog1.end(), expectedSetIntTimes1)); + const cse::time_point expectedSetIntTimes2[] = {startTime, startTime + 6 * stepSize}; const auto& setIntLog2 = slave2->get_set_integer_log(); REQUIRE(setIntLog2.size() == 2);