-
-
Notifications
You must be signed in to change notification settings - Fork 487
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
switch to nauty for generating posets #38795
Conversation
Documentation preview for this PR (built with commit 17f72fa; changes) is ready! 🎉 |
d0459a2
to
81bbadf
Compare
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.
LGTM.
This is a very impressive speedup:
|
Actually, there is another issue: |
Great catch! To avoid such oversights in future (not in this pull request!), wouldn't it be better to have intermediate methods that call |
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 |
merci pour les suggestions |
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.
LGTM.
as there is now a direct method in `nauty` and we have added the interface previously ### 📝 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. URL: sagemath#38795 Reported by: Frédéric Chapoton Reviewer(s): David Coudert, Frédéric Chapoton, Martin Rubey
as there is now a direct method in
nauty
and we have added the interface previously📝 Checklist