Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Similarity Augmented Graph Matching #560

Merged
merged 38 commits into from Apr 7, 2021
Merged

Similarity Augmented Graph Matching #560

merged 38 commits into from Apr 7, 2021

Conversation

asaadeldin11
Copy link
Contributor

@asaadeldin11 asaadeldin11 commented Oct 26, 2020

Closes #347
Allows GraphMatch to accept a similarity matrix and perform similarity augmented graph matching.

See notebook for proof of effectiveness

@netlify
Copy link

netlify bot commented Oct 26, 2020

Deploy preview for graspologic ready!

Built with commit 9dd7e34

https://deploy-preview-560--graspologic.netlify.app

@bdpedigo bdpedigo added this to Not ready for review in PRs Oct 26, 2020
@asaadeldin11 asaadeldin11 marked this pull request as ready for review November 30, 2020 16:20
@asaadeldin11 asaadeldin11 moved this from Not Ready for Review to Ready for Review in PRs Nov 30, 2020
@asaadeldin11
Copy link
Contributor Author

@dwaynepryce @bdpedigo @alyakin314 @Nyecarr @j1c i think this is ready for review if anyone has time to give it a look. Thanks!

@nicaurvi nicaurvi self-requested a review February 1, 2021 19:42
Copy link
Contributor

@nicaurvi nicaurvi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave the math verification on the JHU side but looks good to me other than the documentation comment :)

graspologic/match/gmp.py Outdated Show resolved Hide resolved
graspologic/match/gmp.py Outdated Show resolved Hide resolved
graspologic/match/gmp.py Outdated Show resolved Hide resolved
tests/test_match.py Outdated Show resolved Hide resolved
@bdpedigo
Copy link
Collaborator

@asaadeldin11 can you resolve all open comments so we can merge? would like to use this for something

@bdpedigo
Copy link
Collaborator

looks like black failing

@asaadeldin11
Copy link
Contributor Author

looks like black failing

should be resolved now

graspologic/match/gmp.py Outdated Show resolved Hide resolved
graspologic/match/gmp.py Outdated Show resolved Hide resolved
graspologic/match/gmp.py Outdated Show resolved Hide resolved
graspologic/match/gmp.py Show resolved Hide resolved
tests/test_match.py Outdated Show resolved Hide resolved
@bdpedigo
Copy link
Collaborator

bdpedigo commented Apr 5, 2021

@asaadeldin11 can you resolve conflicts and resolve any comments above that you addressed

graspologic/match/gmp.py Outdated Show resolved Hide resolved
graspologic/match/gmp.py Outdated Show resolved Hide resolved
graspologic/match/qap.py Outdated Show resolved Hide resolved
@bdpedigo bdpedigo self-requested a review April 5, 2021 13:57
@bdpedigo
Copy link
Collaborator

bdpedigo commented Apr 5, 2021

@asaadeldin11 tests failing

@asaadeldin11
Copy link
Contributor Author

failing tests should be resolved

@bdpedigo bdpedigo merged commit 9aaca90 into dev Apr 7, 2021
PRs automation moved this from Ready for Review to Done Apr 7, 2021
@bdpedigo bdpedigo deleted the sim_aug branch April 7, 2021 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
PRs
Done
Development

Successfully merging this pull request may close these issues.

similarity augmented graph matching
3 participants