-
-
Notifications
You must be signed in to change notification settings - Fork 482
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
Created functions to generate random k-tree and partial k-tree graphs #36587
Conversation
For your information, there is an (old) open issue about generating k-trees #11369. |
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.
Thank you for this contribution. Random k-trees are useful.
I have a few suggestions for improving the code.
Thanks for reviewing @dcoudert. Just added another commit implementing your 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.
a few more suggestions.
Thanks, some good ideas. I've added to the Pull Request |
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.
some details.
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.
|
Nauty has a generator of k-trees. We add an interface to this generator. This is a complement of sagemath#36587. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36924 Reported by: David Coudert Reviewer(s): Matthias Köppe
Nauty has a generator of k-trees. We add an interface to this generator. This is a complement of sagemath#36587. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36924 Reported by: David Coudert Reviewer(s): Matthias Köppe
You should certainly rebase the branch on the last beta to launch all the tests. |
e745c71
to
92d5967
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.
Some improvements to ensure that the method cannot be called with wrong parameters.
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.
I don't understand the last 2 commits. Is it due to the last beta release ? You should certainly rebase the branch on the last beta. |
e4ddcb0
to
aabfa65
Compare
aabfa65
to
f1342ec
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.
I have no more comments. Thanks.
LGTM.
…tial k-tree graphs <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> Implemented graph generators which produce a random k-tree and a random partial k-tree. The k-tree algorithm first generates a complete graph, before adding vertices one by one, creating a clique utilising random existing vertices and the new vertex. Partial k-trees are generated by producing a k-tree and then randomly removing edges from it. <!-- Why is this change required? What problem does it solve? --> Previously SageMath lacked a way to generat randomised graphs which have a fixed treewidth value. This provides a methodology to do so . <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> Addresses sagemath#36586 <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36587 Reported by: Damian Basso Reviewer(s): Damian Basso, David Coudert
…tial k-tree graphs <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> Implemented graph generators which produce a random k-tree and a random partial k-tree. The k-tree algorithm first generates a complete graph, before adding vertices one by one, creating a clique utilising random existing vertices and the new vertex. Partial k-trees are generated by producing a k-tree and then randomly removing edges from it. <!-- Why is this change required? What problem does it solve? --> Previously SageMath lacked a way to generat randomised graphs which have a fixed treewidth value. This provides a methodology to do so . <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> Addresses sagemath#36586 <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation accordingly. <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36587 Reported by: Damian Basso Reviewer(s): Damian Basso, David Coudert
Implemented graph generators which produce a random k-tree and a random partial k-tree. The k-tree algorithm first generates a complete graph, before adding vertices one by one, creating a clique utilising random existing vertices and the new vertex. Partial k-trees are generated by producing a k-tree and then randomly removing edges from it.
Previously SageMath lacked a way to generat randomised graphs which have a fixed treewidth value. This provides a methodology to do so .
Addresses #36586
📝 Checklist