-
Notifications
You must be signed in to change notification settings - Fork 9
Fix offsets computation for messages/potentials; Support AND factor #126
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after updating the docstrings
Codecov Report
@@ Coverage Diff @@
## master #126 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 857 856 -1
=========================================
- Hits 857 856 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor
] | ||
) | ||
num_parents = len(self.variables) - 1 | ||
relevant_state = (-self.edge_states_offset + 1) / 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
relevant_state = (-self.edge_states_offset + 1) / 2 | |
relevant_state = (-self.edge_states_offset + 1) // 2 |
parents_edge_states[ii, 1] contains the message index of the parent variable's relevant state. | ||
For ORFactors the relevant state is 0, for ANDFactors the relevant state is 1. | ||
Both indices only take into account the LogicalFactors of the FactorGraph | ||
The parent variable's other state is parents_edge_states[ii, 2] + edge_states_offset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parent variable's other state is parents_edge_states[ii, 2] + edge_states_offset | |
The parent variable's other state is parents_edge_states[ii, 1] + edge_states_offset |
The messages and potentials offsets for factors/factor groups should not be done by type, since they are only used in updating messages/potentials which works with the whole flat array. This PR fixes it.
Also added support for AND factors by slightly tweaking the existing OR factor implementation.
Resolves #125