Skip to content
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

Correct method is_sparse_paving #39382

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

gmou3
Copy link
Contributor

@gmou3 gmou3 commented Jan 27, 2025

It was brought to my attention that the method is_sparse_paving is incorrect. See #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.

@dimpase
Copy link
Member

dimpase commented Jan 27, 2025

Couldn't we just check that both M and M* (the dual of M) are paving - that's one of definitions in the literature.
Namely, here: http://dx.doi.org/10.1016/j.ejc.2011.01.016 (this reference should be added regardless)

@dimpase
Copy link
Member

dimpase commented Jan 27, 2025

Whichever definition you use, please add a reference in the usual way, not here.

Copy link

github-actions bot commented Jan 27, 2025

Documentation preview for this PR (built with commit d894970; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@tscrim
Copy link
Collaborator

tscrim commented Jan 27, 2025

I agree with Dima's suggestion. Also, don't forget the # optional - matroid_database tag.

@gmou3
Copy link
Contributor Author

gmou3 commented Jan 27, 2025

Couldn't we just check that both M and M* (the dual of M) are paving - that's one of definitions in the literature. Namely, here: http://dx.doi.org/10.1016/j.ejc.2011.01.016 (this reference should be added regardless)

Yes, we can. I tried to use an alternative characterization to avoid computation on the dual matroid (whose rank may be much larger). See the first example in the TESTS block.

I agree with Dima's suggestion. Also, don't forget the # optional - matroid_database tag.

Thanks.

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 4, 2025
    
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants