-
-
Notifications
You must be signed in to change notification settings - Fork 454
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
Implementation of algorithm for exact counting graph homomorphisms #38062
base: develop
Are you sure you want to change the base?
Conversation
Documentation preview for this PR (built with commit ffd345d; changes) is ready! 🎉 |
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.
This PR requires some work before. It must be properly documented, doctests are needed, etc. Here is a first list of needed changes.
I don't think we need this helper_functions.py
file. I fact, I'm in favor of using a single file called homomorphisms.py
and gathering all methods related to graph homomorphisms.
""" | ||
# Whether it's BFS or DFS, every node below join node(s) would be | ||
# computed already, so we can go bottom-up safely. | ||
for node in reversed(self.dir_labelled_TD.vertices()): |
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.
Can you clarify the order in which you want to visit the vertices. Here, you take the reverse order of the list returned by vertices()
but vertices()
does not ensure any kind of ordering.
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.
I just read the implementation code for vertices()
, since self.dir_labelled_TD
is a DAG, I assume in this case, the vertices returned would/should be from the source to the sink(s)?
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.
No, we cannot make any assumption on this ordering. It depends on internal data structures and not on the orientation of the graph. If you need a specific ordering, you must use an appropriate graph traversal method (BFS, DFS, etc.).
|
||
def extract_bag_vertex(mapping, index, graph_size): | ||
r""" | ||
Extract the bag vertex at `index` from `mapping` |
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 method don't extract
but returns an index
|
||
def add_vertex_into_mapping(new_vertex, mapping, index, graph_size): | ||
r""" | ||
Insert `new_vertex` at `index` into `mapping` |
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 method don't insert
but returns the index where to do so.
Thank you for all the reviews! Of course, many tests and much documentation are needed.
I also consider the possibility of having only one file, but wouldn't it make the main file I think the name |
We already have files with 20.000+ lines. |
|
||
if target_density >= self.density_threshold: | ||
target = self.actual_target_graph.adjacency_matrix( | ||
vertices=self.actual_target_graph.vertices() |
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.
I think it's best if you fix the order in which you want to consider the vertices, assign it to a list and use it. Otherwise you are not sure what may happen.
""" | ||
# Whether it's BFS or DFS, every node below join node(s) would be | ||
# computed already, so we can go bottom-up safely. | ||
for node in reversed(list(self.dir_labelled_TD.breadth_first_search(min(self.dir_labelled_TD)))): |
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.
Are you sure that min(self.dir_labelled_TD)
is the root ? isn't it safer to search for a vertex with in-degree 0 ?
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.
Are you sure that min(self.dir_labelled_TD) is the root ?
By construction, yes.
…isms functionality
In the past few months, we have been working towards a SageMath library for exact counting graph homomorphisms, based on the proof of Prop. 1.6 from "Homomorphisms Are a Good Basis for Counting Small Subgraphs" by Radu Curticapean, Holger Dell, and Dániel Marx: The algorithm is deterministic and runs in time$\exp(O(k)) + poly(k) \cdot n^{tw(H) + 1}$ for counting the number of homomorphisms from graph $H$ to graph $G$ , where $k = |V(H)|$ , $n = |V(G)|$ , and $tw(H)$ is the treewidth of $H$ .
The library is recently released on GitHub: https://github.com/guojing0/count-graph-homs
This commit is mainly for the sake of CI. More documentations and tests will follow.
📝 Checklist
⌛ Dependencies