WIP: Added TX branching measurement #106
Conversation
| @@ -101,6 +102,7 @@ def test_measurements_body(self): | |||
| self.assertAlmostEqual(connectivity2_abs, m.extensiveness, 3) | |||
| # self.assertAlmostEqual(connectivity3, m., 3) | |||
| self.assertAlmostEqual(connectivity4, m.branching_modules_count, 3) | |||
| self.assertAlmostEqual(connectivity5, m.tx_branching_modules_count, 3) | |||
There was a problem hiding this comment.
because these are floating point values
There was a problem hiding this comment.
Why is a counter a floating point value, can you count half branching modules?
There was a problem hiding this comment.
good point, this is an integer counter :/
There was a problem hiding this comment.
Bit confused about the is folding function:
Why the overwriting in CoreModule
I would think that it is not necessary to iterate the all the children to figure out if it is a folding point.
Check if the module has two childs, check that they are not opposite.
But maybe I am too short-sighted on this.
| if child is None: | ||
| continue | ||
| # ignore TouchSensor and BrickSensor modules | ||
| if isinstance(child, TouchSensorModule) or \ |
There was a problem hiding this comment.
You could inherit TouchSensorModule from SensorModule (create new class) and inherit BrickSensorModule from SensorModule so you only have to do one check instead of two.
There was a problem hiding this comment.
yeah, good point. But I'm not going to touch that right now. It's out of scope and I'm afraid I'm going to break stuff
There was a problem hiding this comment.
Could literaly do
class SensorModule:
pass
class TouchSensorModule(SensorModule):
....
class BrickSensorModule(SensorModule):
...
But yeah, that is up to you.
| else: | ||
| # More than 2 children, this is not a fold | ||
| return False | ||
| if parent_slot is None or target_slot is None: |
There was a problem hiding this comment.
This will always trigger when only the parent_slot is filled in the first iteration that a viable child is set as the parent_slot at line 348.
There was a problem hiding this comment.
What do you mean? This if will trigger if I've found both a first and a second child, at least that was my intention.
There was a problem hiding this comment.
Let's say we are in the first iteration of this for loop and it found a viable child and we are saving the slot of this child at line 348. Then now we come to 354 and we say that this statement is true, since the target slot is still unfilled, then we return false, I do not think this is expected behavior.
| return False | ||
| if parent_slot is None or target_slot is None: | ||
| return False | ||
| return parent_slot.opposite() != target_slot |
There was a problem hiding this comment.
This will only trigger after executing line 350 is executed, but this does not necessarily need to be opposite right? After that it cannot be changed since you already return that it is False.
Overall very confused by this entire function.
| # The CoreModule does not have a parent module | ||
| return super(CoreModule, self).count_module_connections() - 1 | ||
|
|
||
| def is_folding(self) -> bool: |
There was a problem hiding this comment.
Why does is_folding need to be overwritten by CoreModule?
There was a problem hiding this comment.
Because in the other function I assume you always have one connection already, which is the connection to the parent. In this case the assumption is broken.
Adding new measurements