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

Added option for more efficient graph matching matrix operations #1046

Merged
merged 10 commits into from
Aug 7, 2023

Conversation

bkj
Copy link
Contributor

@bkj bkj commented Jul 19, 2023

Reference Issues/PRs

N/A

What does this implement/fix? Briefly explain your changes.

Changed internal match/solver.py code to:

  • avoid repeated matrix multiplications
  • compute trace(X @ Y) more efficiently

Any other comments?

The modified implementation definitely avoids unnecessary computation. Benchmarking this algorithm is hard (very data dependent) so YMMV, but these changes give a 1.5-2x speedup for the settings that I tested.

Additional Information

  • No additional dependencies
  • No API changes
  • Have not build the documentation - only modified internal functions that are not documented

@bdpedigo bdpedigo self-requested a review July 20, 2023 19:32
@bdpedigo bdpedigo changed the title More efficient graph matching matrix operations Added option for more efficient graph matching matrix operations Jul 20, 2023
@bdpedigo
Copy link
Collaborator

@bkj just a reminder about the CLA bot, I think everything else is good to go!

@bkj
Copy link
Contributor Author

bkj commented Aug 7, 2023

How to accept CLA?

https://github.com/microsoft/graspologic/blob/dev/CONTRIBUTING.md says the CLA-bot will comment, but not seeing that

@bkj
Copy link
Contributor Author

bkj commented Aug 7, 2023

@microsoft-github-policy-service agree

@bdpedigo bdpedigo merged commit ba74fdb into graspologic-org:dev Aug 7, 2023
bdpedigo added a commit that referenced this pull request Sep 29, 2023
* Added Python 3.11 (#1039)

* add python 3.11

* fix numba dep warning

* add release note entry (#1040)

* Updated heatmap (#750)

* test pytest ini

* add binary_heatmap, updates to tutorial notebook

* test plotting

* re-add pytest.ini

* Update plot.py

Fix a few bugs

* add new tests

* Update setup.cfg

* remove binary_heatmap

* mpl version req back to normal

* remove binary_heatmap import

* black

* formatting fix

---------

Co-authored-by: Benjamin Pedigo <benjamindpedigo@gmail.com>

* Fixed bugs in type specifications from Numpy 1.25 release (#1047)

* attempted fix for matrix type erroring

* fix spec of csr_array

* add a random seed

* remove isspmatrix

* fix a type check

* black

* Added option for more efficient graph matching matrix operations (#1046)

* more efficient graph matching matrix operations

* fix formatting

* remove an unused import (unrelated)

* expose fast kwarg to user, docs

* run black formatter

* add typehint

* fix mypy

---------

Co-authored-by: bkj <ben@jataware.com>
Co-authored-by: bdpedigo <benjamindpedigo@gmail.com>

* Added an `ax` argument for `screeplot`

* Add ax argument to screeplot (now returns None)

* add a return value for screeplot()

* Refactored for best practices

* Fixed Matplotlib 3.8 compatibility issues (#1049)

* specify angle as kwarg

* try removing some maybe unnecessary code?

* fight with mypy

* black

* fix sorts

* try switch to explain

* fix intersphinx

* other instances of wrong tutorial intersphinx

* 3.3.0 release prep (#1050)

* add release notes

* bump version

---------

Co-authored-by: Alex Loftus <alexloftus2004@gmail.com>
Co-authored-by: Ben Johnson <bkj.322@gmail.com>
Co-authored-by: bkj <ben@jataware.com>
Co-authored-by: Prajwal Agrawal <61898798+kidkoder432@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants