Add is_(sparse_)paving() and automorphism_group()#36962
Add is_(sparse_)paving() and automorphism_group()#36962vbraun merged 2 commits intosagemath:developfrom
Conversation
tscrim
left a comment
There was a problem hiding this comment.
Just a few small doc changes. Otherwise LGTM.
|
Thanks. Can you squish those commits into one with at least slightly more useful message (e.g., "changes suggested by the reviewer")? Then this will be a positive review. <rant> I really hate GH's way of incorporating reviewer changes. The commits are usually far too small, the message is completely generic without saying why these were being changed. I want to apply copious amounts of fire and holy water to it and then launch it into the sun. </rant> |
0ee3317 to
e34645b
Compare
|
Documentation preview for this PR (built with commit e34645b; changes) is ready! 🎉 |
sagemathgh-36962: Add is_(sparse_)paving() and automorphism_group() <!-- ^^^^^ 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 --> Added three functions to the main matroid class. <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> These correspond to two connectivity check algorithms, `is_paving()` and `is_sparse_paving()`, and a miscellaneous function `automorphism_group()`. These are all standard and regularly used notions in matroid theory. ### 📝 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. - [ ] 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#36962 Reported by: gmou3 Reviewer(s): Travis Scrimshaw
sagemathgh-36962: Add is_(sparse_)paving() and automorphism_group() <!-- ^^^^^ 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 --> Added three functions to the main matroid class. <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> These correspond to two connectivity check algorithms, `is_paving()` and `is_sparse_paving()`, and a miscellaneous function `automorphism_group()`. These are all standard and regularly used notions in matroid theory. ### 📝 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. - [ ] 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#36962 Reported by: gmou3 Reviewer(s): Travis Scrimshaw
|
@gmou3 - where do you get this definition of sparse paving? People complain it's not correct, and I don't see any references to this added here. |
|
There is also a missing reference to |
sagemathgh-39382: Correct method `is_sparse_paving` It was brought to my attention that the method `is_sparse_paving` is incorrect. See sagemath#36962 (comment). The method should only check the symmetric differences of `r`-element circuits rather than all (`r`- and `r+1`-element) circuits. The algorithm used is based on a somewhat unusual definition which can be found in https://arxiv.org/pdf/math/0404200. URL: sagemath#39382 Reported by: gmou3 Reviewer(s): Travis Scrimshaw
sagemathgh-39382: Correct method `is_sparse_paving` It was brought to my attention that the method `is_sparse_paving` is incorrect. See sagemath#36962 (comment). The method should only check the symmetric differences of `r`-element circuits rather than all (`r`- and `r+1`-element) circuits. The algorithm used is based on a somewhat unusual definition which can be found in https://arxiv.org/pdf/math/0404200. URL: sagemath#39382 Reported by: gmou3 Reviewer(s): Travis Scrimshaw
sagemathgh-39382: Correct method `is_sparse_paving` It was brought to my attention that the method `is_sparse_paving` is incorrect. See sagemath#36962 (comment). The method should only check the symmetric differences of `r`-element circuits rather than all (`r`- and `r+1`-element) circuits. The algorithm used is based on a somewhat unusual definition which can be found in https://arxiv.org/pdf/math/0404200. URL: sagemath#39382 Reported by: gmou3 Reviewer(s): Travis Scrimshaw
sagemathgh-39382: Correct method `is_sparse_paving` It was brought to my attention that the method `is_sparse_paving` is incorrect. See sagemath#36962 (comment). The method should only check the symmetric differences of `r`-element circuits rather than all (`r`- and `r+1`-element) circuits. The algorithm used is based on a somewhat unusual definition which can be found in https://arxiv.org/pdf/math/0404200. URL: sagemath#39382 Reported by: gmou3 Reviewer(s): Travis Scrimshaw
Added three functions to the main matroid class.
These correspond to two connectivity check algorithms,
is_paving()andis_sparse_paving(), and a miscellaneous functionautomorphism_group(). These are all standard and regularly used notions in matroid theory.📝 Checklist
⌛ Dependencies