Improve performance of graph automorphism via faster refinement and r…#38
Improve performance of graph automorphism via faster refinement and r…#38randomir merged 2 commits intodwavesystems:mainfrom
Conversation
| for u_i in self._u_vector: | ||
| for h in u_i: | ||
| f = mult(g, h) | ||
| if (f == self._identity).all(): |
There was a problem hiding this comment.
For large graphs this short-circuit takes longer to check than it ends up saving, hence its removal.
randomir
left a comment
randomir
left a comment
There was a problem hiding this comment.
Looks good, just a few minor suggestions.
I'll defer the algorithm validation to @jackraymond.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #38 +/- ##
==========================================
+ Coverage 90.40% 95.22% +4.81%
==========================================
Files 13 13
Lines 761 900 +139
==========================================
+ Hits 688 857 +169
+ Misses 73 43 -30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Why is github marking these files as changed? Shouldn't be touched, maybe a technicality?
jackraymond
left a comment
jackraymond
left a comment
There was a problem hiding this comment.
Changes look good to me.
There is some issue with the file-comparison here. It marks almost all files as new, but I know only .
I've functionally tested this in the context of AutomorphismComposite with O(10) embeddings each of size 128 (periodic embedded cubic lattices) in Advantage2 and Advantage systems, seeing a significant jump in performance.
| trace: The number of vertices belonging to each color, ordered by color. | ||
| color: A map from each vertex to its color. | ||
| num_colors: The number of unique colors, equivalent to the number | ||
| of cells in the partition. |
There was a problem hiding this comment.
indent.
| of cells in the partition. | |
| of cells in the partition. |
SebastianGitt
force-pushed
the
main
branch
3 times, most recently
from
c56e1d9 to
401c050
Compare
| color class containing that vertex needs to be placed on the refinement | ||
| stack initially, since only colors adjacent to that color can be affected. | ||
|
|
||
| For performance reasons, `num_colors` is passed as a single-element list to |
There was a problem hiding this comment.
| For performance reasons, `num_colors` is passed as a single-element list to | |
| For performance reasons, ``num_colors`` is passed as a single-element list to |
| stack initially, since only colors adjacent to that color can be affected. | ||
|
|
||
| For performance reasons, `num_colors` is passed as a single-element list to | ||
| `_split_up_color()` so that updates to the number of colors persist across |
There was a problem hiding this comment.
Generally, I would try not to refer to hidden methods in docstrings, but this is also in a hidden method so not that important here. Still, I think it's better to update this reference.
| in_refine_stack: bytearray, | ||
| num_colors: list[int], | ||
| ) -> None: | ||
| """A subroutine of ``_refine()`` that splits a color class into subcells |
There was a problem hiding this comment.
Same here w.r.t. hidden method reference.
| self, | ||
| partition: list[set[int]], | ||
| trace: NDArray[np.integer], | ||
| color: NDArray[np.integer], | ||
| num_colors: int, | ||
| individualized_vertex: Optional[int] = None, |
There was a problem hiding this comment.
| self, | |
| partition: list[set[int]], | |
| trace: NDArray[np.integer], | |
| color: NDArray[np.integer], | |
| num_colors: int, | |
| individualized_vertex: Optional[int] = None, | |
| self, | |
| partition: list[set[int]], | |
| trace: NDArray[np.integer], | |
| color: NDArray[np.integer], | |
| num_colors: int, | |
| individualized_vertex: Optional[int] = None, |
There was a problem hiding this comment.
We can also start using int | None instead of Optional[int] if we'd like. IMO slightly cleaner, and I think we're leaning towards that elsewhere.
…e()`` - uses an optimized colour refinement algorithm - more efficiently handles graphs with disjoint components - fixes issue where automorphisms could be missed if coset representatives could belong to multiple left transversals - supports canonical labelling - faster graph comparison using refinement trace - no longer relies on ``change_base()`` to perform pruning by automorphism - uses more efficient data structures - now returns mapping to original graph labelling - improves test coverage of edge cases - caches coset representative inverses
5 participants
…emoval of
change_base()change_base()to perform pruning by automorphism