Fixed bug in the matroid realization space code - #6207
Open
LukasKuehne wants to merge 4 commits into
Open
Conversation
…on can kill the saturated ideal.
antonydellavecchia
requested changes
Sep 1, 2026
antonydellavecchia
left a comment
Collaborator
There was a problem hiding this comment.
looks good for the most part I would just make sure to use the accesor functions
Co-authored-by: antonydellavecchia <antonydellavecchia@gmail.com>
Contributor
Author
|
Thanks for the comment. I now edited the code to use the access functions consistently. |
antonydellavecchia
approved these changes
Sep 1, 2026
…e-after-reduce # Conflicts: # experimental/MatroidRealizationSpaces/test/runtests.jl
Contributor
Author
|
The last commit just fixes the small merge conflict in the runtests. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The reduction pass that runs after saturation can destroy saturatedness:
eliminating a variable via a solution with a nontrivial denominator (a
unit on the localized space, certified through the inequations) clears
that denominator into the substituted generators, and the resulting
ideal can pick up components supported inside its vanishing locus. The
chart stays correct as a localized scheme, but the returned defining
ideal is then not saturated, contrary to what saturate=true promises —
on the matroid in the added test, a non-basis minor of the returned
matrix does not even lie in the returned ideal.
Fix: alternate reduction and stepwise saturation until the reduction
stabilizes; saturation runs only after a reduction that eliminated
variables, and each productive reduction strictly decreases the
variable count, so the loop terminates.
I added a test case which failed before and now works fine.