Information and Network Architectures #903
Conversation
…tyle, background colour
spike-dstl
force-pushed
the
architecture_repr
branch
from
9c2cba7 to
ffd7fc1
Compare
| doc="An Edges object containing all edges. For A to be connected to B we would have an " | ||
| "Edge with edge_pair=(A, B) in this object.") | ||
| current_time: datetime = Property( | ||
| default=datetime.now(), |
There was a problem hiding this comment.
If current_time is None this is set to datetime.now() in the __init__ method, but the default value is still datetime.now(). Is this correct?
| for leaf_node in self.leaf_nodes: | ||
| try: | ||
| shortest_path = self.shortest_path_dict[leaf_node][node] | ||
| if node != leaf_node or shortest_path != 0: |
There was a problem hiding this comment.
Is there a scenario where one of node != leaf_node or shortest_path != 0 is true but not the other?
| else: | ||
| return 1 | ||
| except KeyError: | ||
| non_leaves += 1 |
There was a problem hiding this comment.
non_leaves is unused
| def __init__(self, *args, **kwargs): | ||
| super().__init__(*args, **kwargs) | ||
|
|
||
| if any([isinstance(node, RepeaterNode) for node in self.all_nodes]): |
There was a problem hiding this comment.
Does "if len(self.repeater_nodes) != 0:" achieve the same result?
| self.information_arch = InformationArchitecture( | ||
| edges=self.information_architecture_edges, current_time=self.current_time) | ||
| else: | ||
| self.information_arch = InformationArchitecture(self.edges, self.current_time) |
There was a problem hiding this comment.
should self.information_architecture_edges be set to self.edges?
| if edge.recipient not in self.information_arch.all_nodes: | ||
| edge.update_messages(self.current_time, to_network_node=True, | ||
| use_arrival_time=self.use_arrival_time) | ||
| else: | ||
| edge.update_messages(self.current_time, use_arrival_time=self.use_arrival_time) |
There was a problem hiding this comment.
| if edge.recipient not in self.information_arch.all_nodes: | |
| edge.update_messages(self.current_time, to_network_node=True, | |
| use_arrival_time=self.use_arrival_time) | |
| else: | |
| edge.update_messages(self.current_time, use_arrival_time=self.use_arrival_time) | |
| edge.update_messages(self.current_time, use_arrival_time=self.use_arrival_time, | |
| to_network_node=(edge.recipient not in self.information_arch.all_nodes)) |
| if edge.recipient not in self.information_arch.all_nodes: | ||
| edge.update_messages(self.current_time, to_network_node=True, | ||
| use_arrival_time=self.use_arrival_time) | ||
| else: | ||
| edge.update_messages(self.current_time, use_arrival_time=self.use_arrival_time) |
There was a problem hiding this comment.
| if edge.recipient not in self.information_arch.all_nodes: | |
| edge.update_messages(self.current_time, to_network_node=True, | |
| use_arrival_time=self.use_arrival_time) | |
| else: | |
| edge.update_messages(self.current_time, use_arrival_time=self.use_arrival_time) | |
| edge.update_messages(self.current_time, use_arrival_time=self.use_arrival_time, | |
| to_network_node=(edge.recipient not in self.information_arch.all_nodes)) |
| try: | ||
| shortest_path = self.shortest_path_dict[leaf_node][node] | ||
| if node != leaf_node or shortest_path != 0: | ||
| node_leaves.add(leaf_node) | ||
| else: | ||
| return 1 | ||
| except KeyError: | ||
| non_leaves += 1 |
There was a problem hiding this comment.
| try: | |
| shortest_path = self.shortest_path_dict[leaf_node][node] | |
| if node != leaf_node or shortest_path != 0: | |
| node_leaves.add(leaf_node) | |
| else: | |
| return 1 | |
| except KeyError: | |
| non_leaves += 1 | |
| try: | |
| shortest_path = self.shortest_path_dict[leaf_node][node] | |
| except KeyError: | |
| non_leaves += 1 | |
| else: | |
| if node != leaf_node or shortest_path != 0: | |
| node_leaves.add(leaf_node) | |
| else: | |
| return 1 |
An else clause should be used to move code that isn't expected to cause an exception out of the try-except
| metrics: Collection[Collection[MetricGenerator]] = Property(doc="Set of metrics to put in the " | ||
| "table") | ||
|
|
||
| metrics_labels: Collection[str] = Property(doc='List of titles for ', |
There was a problem hiding this comment.
Sentence is unfinished
|
|
||
| class SIAPDiffTableGenerator(Base): | ||
| """ | ||
| Given two sets of metric generators, the SiapDiffTableGenerator returns a table displaying the |
There was a problem hiding this comment.
Is there a reason to specify two sets?
| @@ -135,3 +135,112 @@ def set_default_descriptions(self): | |||
| "SIAP ID Correctness": "Fraction of true objects with correct ID assignment", | |||
| "SIAP ID Ambiguity": "Fraction of true objects with ambiguous ID assignment" | |||
| } | |||
|
|
|||
|
|
|||
| class SIAPDiffTableGenerator(Base): | |||
There was a problem hiding this comment.
Would it be possible for this to inherit from SIAPTableGenerator to save from having to redefine set_default_descriptions and set_default_targets
| def update(self, time_pertaining, time_arrived, data_piece, category, track=None, | ||
| use_arrival_time=False): | ||
| """Updates this :class:`~.Node`'s :attr:`~.data_held` using a new data piece. """ |
There was a problem hiding this comment.
Is this or the other two pngs still being used?
| n_archs: int = Property( | ||
| doc="Tuple containing a minimum and maximum value for the number of routes created in the " | ||
| "network architecture to represent a single edge in the information architecture.", | ||
| default=2) |
There was a problem hiding this comment.
Docstring seems inaccurate
| # Method 3: Identical Information and Network Architectures | ||
| net_arch3 = NetworkArchitecture(edges=info_edges) | ||
| assert net_arch3.information_arch.edges == info_edges | ||
| assert net_arch3.edges == info_edges |
There was a problem hiding this comment.
possibly add "method 4: Providing only network architecture edges" to generate the info edges
| assert (nodes['a'], nodes['b']) in edges_list.edge_list | ||
| assert (nodes['a'], nodes['b']) in edges_list.edge_list |
| # Check node types | ||
| assert len(arch.fusion_nodes) == sum([2, 1]) | ||
| assert len(arch.sensor_nodes) == sum([2, 2]) | ||
|
|
There was a problem hiding this comment.
| assert arch.is_hierarchical |
It would make sense to add a check that the "hierarchical" type generates a hierarchical architecture
|
|
||
| # Test invalid time inputs | ||
| with pytest.raises(TypeError): | ||
| A.update(dt0, dt0, 'faux DataPiece', 'created') |
There was a problem hiding this comment.
this test should included a valid data piece to show that its the time causing the fail
Introducing the long-awaited architecture module! Thus far, multi-sensor tracking in Stone Soup has operated on the assumption that data from multiple sensors can be fused together seamlessly, but this addition allows experimentation with different architectures.
As there are a fair chunk of new components, we've provided documentation in the form of three tutorials, which I'd recommend starting with to aid your understanding.