Fix outlier handling in qap when len(seeds)==n #754
Conversation
|
Deploy preview for graspologic ready! Built with commit 3c3e5f0 |
|
thanks for the PR @kellymarchisio, I'll let @asaadeldin11 chime in |
|
although when |
|
@kellymarchisio Thanks for the PR! I think I understand the issue; the score returned is correct, but the |
| @@ -400,6 +400,8 @@ def _quadratic_assignment_faq( | |||
|
|
|||
| # check outlier cases | |||
| if n == 0 or partial_match.shape[0] == n: | |||
| # Cannot assume partial_match is sorted. | |||
| partial_match = np.row_stack(sorted(partial_match, key=lambda x: x[0])) | |||
There was a problem hiding this comment.
does sorted require partial_match to be a list?
There was a problem hiding this comment.
Nope - it can be any iterable.
There was a problem hiding this comment.
great. can you add a test to the function test_barycenter_SGM in test_match.py that gives (a possibly shuffled) full partial_match and returns the correct score_ and perm_inds? (Should be very similar to the test in lines 138-142)
There was a problem hiding this comment.
Sure - done :)
The score calculation is wrong, and then the return value of fit is wrong: ^^ perm_inds are not sorted, as they would be if QAP had run. So looks like both the score and perm_inds are incorrect. You can reproduce this by sending in sorted vs. unsorted seeds of list length n. |
|
|
| chr12c = self.barycenter.fit(A, B, W1, W2) | ||
| score = chr12c.score_ | ||
| assert 11156 == score | ||
|
|
There was a problem hiding this comment.
can you check the perm_inds_ here as well
Co-authored-by: Ali Saad-Eldin <54996865+asaadeldin11@users.noreply.github.com>
…graspologic into outlier-handling
|
@bdpedigo this looks good to me |
Reference Issues/PRs
What does this implement/fix? Briefly explain your changes.
QAP incorrectly assumes the partial_match is sorted when partial_match.shape[0] == A.shape[0]. Fix this.
Any other comments?