diff options
-rw-r--r-- | host/docs/properties.dox | 26 | ||||
-rw-r--r-- | host/lib/include/uhdlib/rfnoc/graph.hpp | 11 | ||||
-rw-r--r-- | host/lib/rfnoc/graph.cpp | 34 | ||||
-rw-r--r-- | host/tests/rfnoc_propprop_test.cpp | 5 |
4 files changed, 43 insertions, 33 deletions
diff --git a/host/docs/properties.dox b/host/docs/properties.dox index 7ab99da3e..a56a94462 100644 --- a/host/docs/properties.dox +++ b/host/docs/properties.dox @@ -345,24 +345,28 @@ Consider the following graph: In this application, we receive a signal, use the DDC block to correct a frequency offset digitally, and then apply some custom DSP before transmitting the signal again, using the same radio block. In order to create a valid graph -that UHD can handle, one of the edges needs to be declared a back edge. +that UHD can handle, one of the edges needs to be declared a back-edge. -Edge properties are not propagated across back edges. However, they must still -match up after property resolution is done. For example, if the DDC were to be +When declaring an edge as a back-edge, this signals to UHD that this edge should +not be used for viewing the graph as a directed, acyclic graph (DAG), which is +important during property propagation. In the graph above, UHD would sort blocks +topologically (Radio Block, DDC Block, Custom DSP Block) by ignoring back-edges. +Then, properties are forwarded in topological order first across forward edges, +then across back edges. + +This can lead to resolution errors. For example, if the DDC were to be configured to decimate the sampling rate, and the custom DSP block would not correct for that, the output edge property `samp_rate` of the custom DSP block would be mismatched compared to the input edge property `samp_rate` of the radio. -In that case, UHD will throw an exception when trying to resolve properties, -even if it is declared a back-edge. +This could either re-trigger a new cascade of property settings, which means +the algorithm can't converge, and UHD would detect that and throw an exception. +Or the radio block might not accept a sample rate update on its input port, +which would also trigger an exception. -Note that property propagation along edges is required for certain operations. -Therefore, it is highly recommended to not declare edges as back-edges unless +It is highly recommended to not declare edges as back-edges unless necessary. In the graph above, any of the three edges could have been declared a back-edge to result in a topologically valid graph that can still resolve its -properties, but the one chosen is the most intuitive selection. If any *two* -edges were declared back-edges, at least one of the DDC or Custom DSP blocks -might cease to function, e.g., because it doesn't have access to the tick rate -property. +properties, but the one chosen is the most intuitive selection. \section props_unknown Handling unknown properties diff --git a/host/lib/include/uhdlib/rfnoc/graph.hpp b/host/lib/include/uhdlib/rfnoc/graph.hpp index 9667f4817..2cbf5fb9d 100644 --- a/host/lib/include/uhdlib/rfnoc/graph.hpp +++ b/host/lib/include/uhdlib/rfnoc/graph.hpp @@ -166,14 +166,21 @@ private: /************************************************************************** * The Algorithm *************************************************************************/ - /*! Implementation of the property propagation algorithm - */ void resolve_all_properties(uhd::rfnoc::resolve_context context, rfnoc_graph_t::vertex_descriptor initial_node); void resolve_all_properties(uhd::rfnoc::resolve_context context, node_ref_t initial_node); + /*! This is the real implementation of the property propagation algorithm. + * + * This method must only be called from resolve_all_properties(). It assumes + * that sanity checks have run, and that the graph mutex is being held. + */ + void _resolve_all_properties(uhd::rfnoc::resolve_context context, + rfnoc_graph_t::vertex_descriptor initial_node, + const bool forward); + /************************************************************************** * Action API *************************************************************************/ diff --git a/host/lib/rfnoc/graph.cpp b/host/lib/rfnoc/graph.cpp index 3584a1c2a..9ce100ec7 100644 --- a/host/lib/rfnoc/graph.cpp +++ b/host/lib/rfnoc/graph.cpp @@ -287,7 +287,6 @@ void graph_t::resolve_all_properties( return; } - node_accessor_t node_accessor{}; // We can't release during property propagation, so we lock this entire // method to make sure that a) different threads can't interfere with each // other, and b) that we don't release the graph while this method is still @@ -297,6 +296,7 @@ void graph_t::resolve_all_properties( return; } if (_release_count) { + node_accessor_t node_accessor{}; node_ref_t current_node = boost::get(vertex_property_t(), _graph, initial_node); UHD_LOG_TRACE(LOG_ID, "Only resolving node " << current_node->get_unique_id() @@ -308,6 +308,19 @@ void graph_t::resolve_all_properties( return; } + UHD_LOG_TRACE(LOG_ID, "Running forward edge property propagation..."); + _resolve_all_properties(context, initial_node, true); + UHD_LOG_TRACE(LOG_ID, "Running backward edge property propagation..."); + _resolve_all_properties(context, initial_node, false); +} + + +void graph_t::_resolve_all_properties(resolve_context context, + rfnoc_graph_t::vertex_descriptor initial_node, + const bool forward) +{ + node_accessor_t node_accessor{}; + // First, find the node on which we'll start. auto initial_dirty_nodes = _find_dirty_nodes(); if (initial_dirty_nodes.size() > 1) { @@ -364,7 +377,7 @@ void graph_t::resolve_all_properties( // Forward all edge props in all directions from current node. We make // sure to skip properties if the edge is flagged as // !property_propagation_active - _forward_edge_props(*node_it, true); + _forward_edge_props(*node_it, forward); // Now mark all properties on this node as clean node_accessor.clean_props(current_node); @@ -412,7 +425,7 @@ void graph_t::resolve_all_properties( } // Post-iteration sanity checks: - // First, we make sure that there are no dirty properties left. If there are, + // Make sure that there are no dirty properties left. If there are, // that means our algorithm couldn't converge and we have a problem. auto remaining_dirty_nodes = _find_dirty_nodes(); if (!remaining_dirty_nodes.empty()) { @@ -429,21 +442,6 @@ void graph_t::resolve_all_properties( } throw uhd::resolve_error("Could not resolve properties."); } - - // Second, go through edges marked !property_propagation_active and make - // sure that they match up - BackEdgePredicate back_edge_filter(_graph); - auto e_iterators = - boost::edges(boost::filtered_graph<rfnoc_graph_t, BackEdgePredicate>( - _graph, back_edge_filter)); - bool back_edges_valid = true; - for (auto e_it = e_iterators.first; e_it != e_iterators.second; ++e_it) { - back_edges_valid = back_edges_valid && _assert_edge_props_consistent(*e_it); - } - if (!back_edges_valid) { - throw uhd::resolve_error( - "Error during property resolution: Back-edges inconsistent!"); - } } void graph_t::resolve_all_properties( diff --git a/host/tests/rfnoc_propprop_test.cpp b/host/tests/rfnoc_propprop_test.cpp index 1752e4c21..130137a28 100644 --- a/host/tests/rfnoc_propprop_test.cpp +++ b/host/tests/rfnoc_propprop_test.cpp @@ -385,8 +385,9 @@ BOOST_AUTO_TEST_CASE(test_graph_ro_prop) const size_t rx_rssi_resolver_count = mock_rx_radio.rssi_resolver_count; UHD_LOG_INFO("TEST", "Now testing mock RSSI resolver/get prop"); UHD_LOG_DEBUG("TEST", "RX RSSI: " << mock_rx_radio.get_property<double>("rssi")); - // The next value must match the value in graph.cpp - BOOST_CHECK_EQUAL(rx_rssi_resolver_count + 1, mock_rx_radio.rssi_resolver_count); + // The next value must match the value in graph.cpp. We have one additional + // backward- and forward resolution. + BOOST_CHECK_EQUAL(rx_rssi_resolver_count + 2, mock_rx_radio.rssi_resolver_count); } BOOST_AUTO_TEST_CASE(test_graph_double_connect) |