sage
sage copied to clipboard
switch to nauty for generating posets
as there is now a direct method in nauty and we have added the interface previously
:memo: Checklist
- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation preview.
Documentation preview for this PR (built with commit 17f72fa4080c60a8a2d772ec4c21fd928c7471ed; changes) is ready! :tada: This preview will update shortly after each push to this PR.
This is a very impressive speedup:
sage: %time n=7; sum(1 for dig in DiGraphGenerators()(n, is_poset))
CPU times: user 5.85 s, sys: 0 ns, total: 5.85 s
Wall time: 5.85 s
2045
sage: %time n=7; sum(1 for dig in DiGraphGenerators().nauty_posetg(f"{n} o"))
CPU times: user 57.3 ms, sys: 0 ns, total: 57.3 ms
Wall time: 66.7 ms
2045
sage: %time n=8; sum(1 for dig in DiGraphGenerators().nauty_posetg(f"{n} o"))
CPU times: user 395 ms, sys: 0 ns, total: 395 ms
Wall time: 399 ms
16999
Actually, there is another issue: nauty_posetg works only for $n\leq 16$. You could use a switch to use it only in this case and use the previous (slower) method otherwise.
Actually, there is another issue:
nauty_posetgworks only for n ≤ 16 . You could use a switch to use it only in this case and use the previous (slower) method otherwise.
Great catch!
To avoid such oversights in future (not in this pull request!), wouldn't it be better to have intermediate methods that call nauty_posetg and do the argument parsing? I am frequently wasting time looking up the precise string for nauty_geng to generate connected graphs only :-)
I think that the issue pointed out by @dcoudert still needs fixing.
We lose a lot of speed in the relabeling..
I don't see how to avoid the topological sort. However, you can avoid a copy of the graph
- yield FinitePoset(dig.relabel(label_dict, inplace=False))
+ yield FinitePoset(dig.relabel(label_dict, inplace=True))
note also that instead of dig.order(), you can use self._n. All graphs have the same number of vertices.
merci pour les suggestions