Skip to content

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
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
vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 9, 2025
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
vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 9, 2025
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
@vbraun vbraun merged commit a8fc6bc into sagemath:develop Feb 10, 2025
22 of 23 checks passed
@gmou3 gmou3 deleted the correct_sparse_paving branch February 11, 2025 13:53
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.

4 participants