Skip to content

Commit

Permalink
block setting store/reset defaults
Browse files Browse the repository at this point in the history 
Signed-off-by: Ralph J. Steinhagen <[email protected]>
Signed-off-by: rstein <[email protected]>
  • Loading branch information
@RalphSteinhagen
RalphSteinhagen committed Sep 15, 2023
1 parent 61e2bf8 commit 6791fcd
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 47 deletions.
5 changes: 2 additions & 3 deletions include/node.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,8 @@ struct node : protected std::tuple<Arguments...> {
_output_tags_changed = true;
}

// TODO: expand on this init function:
// * store initial setting -> needed for `reset()` call
// * ...
// store default settings -> can be recovered with 'reset_defaults()'
settings().store_defaults();
}

void
Expand Down
163 changes: 126 additions & 37 deletions include/settings.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,31 @@ struct SettingsCtx {
property_map context; /// user-defined multiplexing context for which the setting is valid
};

/**
* @brief a concept verifying whether a processing block optionally provides a `settings_changed` callback to react to
* block configuration changes and/or to influence forwarded downstream parameters.
*
* Implementers may have:
* 1. `settings_changed(oldSettings, newSettings)`
* 2. `settings_changed(oldSettings, newSettings, forwardSettings)`
* - where `forwardSettings` is for influencing subsequent blocks. E.g., a decimating block might adjust the `sample_rate` for downstream blocks.
*/
template<typename BlockType>
concept HasSettingsChangedCallback = requires(BlockType *node, const property_map &oldSettings, property_map &newSettings) {
{ node->settings_changed(oldSettings, newSettings) };
} or requires(BlockType *node, const property_map &oldSettings, property_map &newSettings, property_map &forwardSettings) {
{ node->settings_changed(oldSettings, newSettings, forwardSettings) };
};

/**
* @brief a concept verifying whether a processing block optionally provides a `reset` callback to react to
* block reset requests (being called after the settings have been reverted(.
*/
template<typename BlockType>
concept HasSettingsResetCallback = requires(BlockType *node) {
{ node->reset() };
};

template<typename T>
concept Settings = requires(T t, std::span<const std::string> parameter_keys, const std::string &parameter_key, const property_map &parameters, SettingsCtx ctx) {
/**
Expand Down Expand Up @@ -110,6 +135,13 @@ struct settings_base {
set(const property_map &parameters, SettingsCtx ctx = {})
= 0;

virtual void
store_defaults()
= 0;
virtual void
reset_defaults()
= 0;

/**
* @brief updates parameters based on node input tags for those with keys stored in `auto_update_parameters()`
* Parameter changes to down-stream nodes is controlled via `auto_forward_parameters()`
Expand Down Expand Up @@ -174,12 +206,22 @@ class basic_settings : public settings_base {
property_map _staged{}; // parameters to become active before the next work() call
std::set<std::string, std::less<>> _auto_update{};
std::set<std::string, std::less<>> _auto_forward{};
property_map _default_settings{};

public:
basic_settings() = delete;
~basic_settings() = default;

explicit constexpr basic_settings(Node &node) noexcept : settings_base(), _node(&node) {
if constexpr (requires { &Node::settings_changed; }) { // if settings_changed is defined
static_assert(HasSettingsChangedCallback<Node>, "if provided, settings_changed must have either a `(const property_map& old, property_map& new, property_map& fwd)`"
"or `(const property_map& old, property_map& new)` paremeter signatures.");
}

if constexpr (requires { &Node::reset; }) { // if reset is defined
static_assert(HasSettingsResetCallback<Node>, "if provided, reset() may have no function parameters");
}

if constexpr (refl::is_reflectable<Node>()) {
meta::tuple_for_each(
[this](auto &&default_tag) {
Expand Down Expand Up @@ -315,6 +357,20 @@ class basic_settings : public settings_base {
return ret; // N.B. returns those <key:value> parameters that could not be set
}

void
store_defaults() override {
this->store_default_settings(_default_settings);
}

void
reset_defaults() override {
_staged = _default_settings;
std::ignore = apply_staged_parameters();
if constexpr (HasSettingsResetCallback<Node>) {
_node->reset();
}
}

void
auto_update(const property_map &parameters, SettingsCtx = {}) override {
if constexpr (refl::is_reflectable<Node>()) {
Expand Down Expand Up @@ -394,38 +450,29 @@ class basic_settings : public settings_base {
if constexpr (refl::is_reflectable<Node>()) {
std::lock_guard lg(_lock);

property_map oldSettings;
if constexpr (
requires(const property_map &cmap, property_map &map) { _node->settings_changed(/* old settings */ cmap, /* new settings */ map); } or //
requires(const property_map &cmap, property_map &map) { _node->settings_changed(/* old settings */ cmap, /* new settings */ map, /* new forward settings */ map); }) {
// take a copy of the field -> map value of the old settings
if constexpr (refl::is_reflectable<Node>()) {
auto iterate_over_member = [&, this](auto member) {
using Type = unwrap_if_wrapped_t<std::remove_cvref_t<decltype(member(*_node))>>;

if constexpr (is_readable(member) && (std::integral<Type> || std::floating_point<Type> || std::is_same_v<Type, std::string> || fair::meta::vector_type<Type>) ) {
oldSettings.insert_or_assign(get_display_name(member), pmtv::pmt(member(*_node)));
}
};
if constexpr (detail::HasBaseType<Node>) {
refl::util::for_each(refl::reflect<typename std::remove_cvref_t<Node>::base_t>().members, iterate_over_member);
}
refl::util::for_each(refl::reflect<Node>().members, iterate_over_member);
}
// prepare old settings if required
property_map oldSettings;
if constexpr (HasSettingsChangedCallback<Node>) {
store_default_settings(oldSettings);
}

// check if reset of settings should be performed
if (_staged.contains(fair::graph::tag::RESET_DEFAULTS)) {
_staged.clear();
reset_defaults();
}

// update staged and forward parameters based on member properties
property_map staged;
for (const auto &[localKey, localStaged_value] : _staged) {
const auto &key = localKey;
const auto &staged_value = localStaged_value;
auto iterate_over_member = [&key, &staged, &forward_parameters, &staged_value, this](auto member) {
const auto &key = localKey;
const auto &staged_value = localStaged_value;
auto apply_member_changes = [&key, &staged, &forward_parameters, &staged_value, this](auto member) {
using Type = unwrap_if_wrapped_t<std::remove_cvref_t<decltype(member(*_node))>>;
if constexpr (is_writable(member) && (std::integral<Type> || std::floating_point<Type> || std::is_same_v<Type, std::string> || fair::meta::vector_type<Type>) ) {
if constexpr (is_writable(member) && is_supported_type<Type>()) {
if (std::string(get_display_name(member)) == key && std::holds_alternative<Type>(staged_value)) {
member(*_node) = std::get<Type>(staged_value);
if constexpr (
requires { _node->settings_changed(/* old settings */ _active, /* new settings */ staged); } or //
requires { _node->settings_changed(/* old settings */ _active, /* new settings */ staged, /* new forward settings */ forward_parameters); }) {
if constexpr (HasSettingsChangedCallback<Node>) {
staged.insert_or_assign(key, staged_value);
} else {
std::ignore = staged; // help clang to see why staged is not unused
Expand All @@ -436,32 +483,40 @@ class basic_settings : public settings_base {
}
}
};
if constexpr (detail::HasBaseType<Node>) {
refl::util::for_each(refl::reflect<typename std::remove_cvref_t<Node>::base_t>().members, iterate_over_member);
}
refl::util::for_each(refl::reflect<Node>().members, iterate_over_member);
process_members<Node>(apply_member_changes);
}
auto iterate_over_member = [&, this](auto member) {
using Type = unwrap_if_wrapped_t<std::remove_cvref_t<decltype(member(*_node))>>;

if constexpr (is_readable(member) && (std::integral<Type> || std::floating_point<Type> || std::is_same_v<Type, std::string> || fair::meta::vector_type<Type>) ) {
// update active parameters
auto update_active = [this](auto member) {
using Type = unwrap_if_wrapped_t<std::remove_cvref_t<decltype(member(*_node))>>;
if constexpr (is_readable(member) && is_supported_type<Type>()) {
_active.insert_or_assign(get_display_name(member), pmtv::pmt(member(*_node)));
}
};
if constexpr (detail::HasBaseType<Node>) {
refl::util::for_each(refl::reflect<typename std::remove_cvref_t<Node>::base_t>().members, iterate_over_member);
}
refl::util::for_each(refl::reflect<Node>().members, iterate_over_member);
process_members<Node>(update_active);

// invoke user-callback function if staged is not empty
if (!staged.empty()) {
if constexpr (requires { _node->settings_changed(/* old settings */ _active, /* new settings */ staged); }) {
_node->settings_changed(/* old settings */ oldSettings, /* new settings */ staged);
} else if constexpr (requires { _node->settings_changed(/* old settings */ _active, /* new settings */ staged, /* new forward settings */ forward_parameters); }) {
_node->settings_changed(/* old settings */ oldSettings, /* new settings */ staged, /* new forward settings */ forward_parameters);
}
}

if (_staged.contains(fair::graph::tag::STORE_DEFAULTS)) {
store_defaults();
}

if constexpr (HasSettingsResetCallback<Node>) {
if (_staged.contains(fair::graph::tag::RESET_DEFAULTS)) {
_node->reset();
}
}

_staged.clear();
}

settings_base::_changed.store(false);
return forward_parameters;
}
Expand All @@ -473,7 +528,7 @@ class basic_settings : public settings_base {
auto iterate_over_member = [&, this](auto member) {
using Type = unwrap_if_wrapped_t<std::remove_cvref_t<decltype(member(*_node))>>;

if constexpr (is_readable(member) && (std::integral<Type> || std::floating_point<Type> || std::is_same_v<Type, std::string>) ) {
if constexpr (is_readable(member) && is_supported_type<Type>()) {
_active.insert_or_assign(get_display_name_const(member).str(), member(*_node));
}
};
Expand All @@ -483,6 +538,40 @@ class basic_settings : public settings_base {
refl::util::for_each(refl::reflect<Node>().members, iterate_over_member);
}
}

private:
void
store_default_settings(property_map &oldSettings) {
// take a copy of the field -> map value of the old settings
if constexpr (refl::is_reflectable<Node>()) {
auto iterate_over_member = [&, this](auto member) {
using Type = unwrap_if_wrapped_t<std::remove_cvref_t<decltype(member(*_node))>>;

if constexpr (is_readable(member) && is_supported_type<Type>()) {
oldSettings.insert_or_assign(get_display_name(member), pmtv::pmt(member(*_node)));
}
};
if constexpr (detail::HasBaseType<Node>) {
refl::util::for_each(refl::reflect<typename std::remove_cvref_t<Node>::base_t>().members, iterate_over_member);
}
refl::util::for_each(refl::reflect<Node>().members, iterate_over_member);
}
}

template<typename Type>
inline constexpr static bool
is_supported_type() {
return std::integral<Type> || std::floating_point<Type> || std::is_same_v<Type, std::string> || fair::meta::vector_type<Type>;
}

template<typename NodeType, typename Func>
inline constexpr static void
process_members(Func func) {
if constexpr (detail::HasBaseType<NodeType>) {
refl::util::for_each(refl::reflect<typename std::remove_cvref_t<NodeType>::base_t>().members, func);
}
refl::util::for_each(refl::reflect<NodeType>().members, func);
}
};

static_assert(Settings<basic_settings<int>>);
Expand Down
13 changes: 7 additions & 6 deletions include/tag.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,16 @@ using property_map = pmtv::map_t;
*/
struct alignas(hardware_constructive_interference_size) tag_t {
using signed_index_type = std::make_signed_t<std::size_t>;
signed_index_type index{0};
property_map map{};
signed_index_type index{ 0 };
property_map map{};

tag_t() = default; // TODO: remove -- needed only for Clang <=15

tag_t(signed_index_type index_, property_map map_) noexcept: index(index_), map(std::move(
map_)) {} // TODO: remove -- needed only for Clang <=15
tag_t(signed_index_type index_, property_map map_) noexcept : index(index_), map(std::move(map_)) {} // TODO: remove -- needed only for Clang <=15

bool
operator==(const tag_t &other) const
= default;
= default;

// TODO: do we need the convenience methods below?
void
Expand Down Expand Up @@ -190,8 +189,10 @@ inline EM_CONSTEXPR_STATIC default_tag<"trigger_name", std::string> TRIGGER_NAME
inline EM_CONSTEXPR_STATIC default_tag<"trigger_time", uint64_t, "ns", "UTC-based time-stamp"> TRIGGER_TIME;
inline EM_CONSTEXPR_STATIC default_tag<"trigger_offset", float, "s", "sample delay w.r.t. the trigger (e.g.compensating analog group delays)"> TRIGGER_OFFSET;
inline EM_CONSTEXPR_STATIC default_tag<"context", std::string, "", "multiplexing key to orchestrate node settings/behavioural changes"> CONTEXT;
inline EM_CONSTEXPR_STATIC default_tag<"reset_default", bool, "", "reset block state to stored default"> RESET_DEFAULTS;
inline EM_CONSTEXPR_STATIC default_tag<"store_default", bool, "", "store block settings as default"> STORE_DEFAULTS;

inline constexpr std::tuple DEFAULT_TAGS = { SAMPLE_RATE, SIGNAL_NAME, SIGNAL_UNIT, SIGNAL_MIN, SIGNAL_MAX, TRIGGER_NAME, TRIGGER_TIME, TRIGGER_OFFSET, CONTEXT };
inline constexpr std::tuple DEFAULT_TAGS = { SAMPLE_RATE, SIGNAL_NAME, SIGNAL_UNIT, SIGNAL_MIN, SIGNAL_MAX, TRIGGER_NAME, TRIGGER_TIME, TRIGGER_OFFSET, CONTEXT, RESET_DEFAULTS, STORE_DEFAULTS };
} // namespace tag

} // namespace fair::graph
Expand Down
42 changes: 41 additions & 1 deletion test/qa_settings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ struct TestBlock : public node<TestBlock<T>, BlockingIO<true>, TestBlockDoc, Sup
std::vector<T> vector_setting{ T(3), T(2), T(1) };
int update_count = 0;
bool debug = false;
bool resetCalled = false;

void
settings_changed(const property_map &old_settings, property_map &new_settings) noexcept {
Expand All @@ -143,6 +144,12 @@ struct TestBlock : public node<TestBlock<T>, BlockingIO<true>, TestBlockDoc, Sup
}
}

void
reset() {
// optional reset function
resetCalled = true;
}

template<fair::meta::t_or_simd<T> V>
[[nodiscard]] constexpr V
process_one(const V &a) const noexcept {
Expand All @@ -165,7 +172,7 @@ struct Decimate : public node<Decimate<T, Average>, SupportedTypes<float, double
void
settings_changed(const property_map & /*old_settings*/, property_map &new_settings, property_map &fwd_settings) noexcept {
if (new_settings.contains(std::string(fair::graph::tag::SIGNAL_RATE.shortKey())) || new_settings.contains("denominator")) {
const float fwdSampleRate = sample_rate / static_cast<float>(this->denominator);
const float fwdSampleRate = sample_rate / static_cast<float>(this->denominator);
fwd_settings[std::string(fair::graph::tag::SIGNAL_RATE.shortKey())] = fwdSampleRate; // TODO: handle 'gr:sample_rate' vs 'sample_rate';
}
}
Expand Down Expand Up @@ -432,6 +439,39 @@ const boost::ut::suite SettingsTests = [] {
expect(eq(block2.sample_rate, 500.0f)) << "block2 matching sample_rate";
expect(eq(sink.sample_rate, 100.0f)) << "sink matching src sample_rate";
};

"basic store/reset settings"_test = []() {
graph flow_graph;
auto &block = flow_graph.make_node<TestBlock<float>>({ { "name", "TestName" }, { "scaling_factor", 2.f } });
expect(block.name == "TestName");
expect(eq(block.scaling_factor, 2.f));

std::ignore = block.settings().set({ { "name", "TestNameAlt" }, { "scaling_factor", 42.f } });
std::ignore = block.settings().apply_staged_parameters();
expect(block.name == "TestNameAlt");
expect(eq(block.scaling_factor, 42.f));

expect(not block.resetCalled);
block.settings().reset_defaults();
expect(block.resetCalled);
block.resetCalled = false;
expect(block.name == "TestName");
expect(eq(block.scaling_factor, 2.f));

std::ignore = block.settings().set({ { "name", "TestNameAlt" }, { "scaling_factor", 42.f } });
std::ignore = block.settings().apply_staged_parameters();
expect(block.name == "TestNameAlt");
expect(eq(block.scaling_factor, 42.f));
block.settings().store_defaults();
std::ignore = block.settings().set({ { "name", "TestNameAlt2" }, { "scaling_factor", 43.f } });
std::ignore = block.settings().apply_staged_parameters();
expect(block.name == "TestNameAlt2");
expect(eq(block.scaling_factor, 43.f));
block.settings().reset_defaults();
expect(block.resetCalled);
expect(block.name == "TestNameAlt");
expect(eq(block.scaling_factor, 42.f));
};
};

const boost::ut::suite AnnotationTests = [] {
Expand Down

0 comments on commit 6791fcd

Please sign in to comment.