Cache is_projective_planar() method for graphs #40987
Conversation
orlitzky
changed the title
|
Documentation preview for this PR (built with commit 962a584; changes) is ready! 🎉 |
orlitzky
force-pushed
the
cached-is-projective-planar
branch
from
79df099 to
7cecec2
Compare
|
A simpler solution is to use a different graph, for instance: sage: G = graphs.CompleteGraph(5)
sage: %time G.is_projective_planar()
CPU times: user 1.16 ms, sys: 16 µs, total: 1.18 ms
Wall time: 1.18 ms
True
sage: G = graphs.CompleteGraph(6)
sage: %time G.is_projective_planar()
CPU times: user 1.15 ms, sys: 51 µs, total: 1.2 ms
Wall time: 1.21 ms
True
sage: G = graphs.CompleteGraph(7)
sage: %time G.is_projective_planar()
CPU times: user 124 ms, sys: 2.01 ms, total: 126 ms
Wall time: 125 ms
Falseand to mark the test for the Petersen Graph as There is a typo |
Would you prefer to do it that way? Using a cache has the benefit that the "computation" really is faster if anyone actually tries to use it, but we could keep the pytest test either way, so that's really the only difference. |
|
I agree that the cache option is nice, so keep it as is. We may add a faster test with a clique. Also, it might be better to create a subdirectory |
|
Done, just need to wait for the CI to confirm that it still gets run from the subdirectory. |
|
This seems ok. I tested it on my laptop. |
The is_projective_planar() function can be very slow. The first example in the documentation is the Petersen graph, on which the method is taking over six minutes to arrive at the answer (True). We mark it as a @cached_method, since that will allow us to pre- fill the cache for graphs that are known to be projective planar, saving the user a lot of time.
The Petersen graph is known to be projective planar, so we can pre-fill the cache with "True" when it is constructed.
…ests To ensure that is_projective_planar() still works in the absense of the cache (now that it is a @cached_method), we add a new pytest file. For the moment it only tests the Petersen graph, since that is the only graph with the method initially cached.
Ultimately we may not want to install these test files, but for now the CI is complaining about it. We can decide later on.
A simple `git mv` with an update of the associated meson.build files.
The "tests" subdirectory is installed, so make it a package for consistency. Add the generated __init__.py to .gitignore while we're at it.
orlitzky
force-pushed
the
cached-is-projective-planar
branch
from
47fca2e to
962a584
Compare
I'm pretty sure that moving a file didn't cause all of those "illegal instruction" crashes, but there's a new beta out anyway so I took the opportunity to rebase and re-run the CI. |
The main example of this (the Petersen graph) is taking over six minutes on the CI, and even longer on my machine. We already know the answer when the graph is constructed, so by making it a
@cached_method, the method becomes effectively instantaneous.We still want to test the computation, but that can be done via pytest (and now is) rather than in a user-facing example.