gh-40301: Fix submodule containment over polynomial rings#42171
Open
cxzhong wants to merge 3 commits into
Open
gh-40301: Fix submodule containment over polynomial rings#42171cxzhong wants to merge 3 commits into
cxzhong wants to merge 3 commits into
Conversation
Issue: Containment test "vec in G" returned True for vectors not in G when the base ring was a multivariate polynomial ring (and several other cases), because the existing fallback element constructor in Module_free_ambient performed no actual membership check. This commit completes and extends the prior partial fix: 1. free_module.py: in Module_free_ambient._element_constructor_, always call _check_element_membership (not only over exact coordinate rings). Inexact rings such as CC[x,y] previously skipped the check entirely, leading to the original sagemath#40301 bug for the zero submodule. 2. free_module.py: guard is_submodule against infinite recursion when self.ambient_module() is self, which happens for QuotientModule instances and caused stack overflows for subquotients. 3. submodule.py: rework Submodule_free_ambient._groebner_basis_contains so that - zero submodules are decided directly (works over any ring, including inexact ones), - univariate polynomial rings are wrapped into libsingular multivariate rings with one variable so that Singular can compute module Groebner bases (fixes ZZ[x], number field polynomial rings, etc.), - quotient module ambients (subquotients) include the relations of the quotient as additional generators, - Singular failures over unsupported coefficient rings now raise NotImplementedError so that the calling code falls back gracefully. 4. submodule.py: override __contains__ in Submodule_free_ambient to use the membership check directly instead of relying on cross-parent equality (no coercion is registered between a submodule of a quotient module and the quotient itself). 5. Added regression doctests covering the original bug example, the ZZ[x] case from the issue comments, the inexact-ring zero submodule case, and the quotient module subquotient case. Fixes sagemath#40301.
…tainment Expand the docstring of Submodule_free_ambient._groebner_basis_contains to document and test nonzero submodules over * univariate polynomial rings that are not PIDs (e.g. ZZ[x]), including ZZ[x]-linear combinations and the ZZ[x] vs QQ(x) distinction; * multivariate polynomial rings over ZZ (e.g. ZZ[x, y]), exercising integer multiples, polynomial-coefficient combinations, and the rejection of (1, 1) which lies in the QQ(x, y)-span but not in the ZZ[x, y]-span.
… over CC[] Add doctests to Submodule_free_ambient._groebner_basis_contains showing that, over inexact coefficient rings such as CC[x, y], nonzero submodule containment queries correctly raise NotImplementedError instead of returning a possibly wrong answer. This is exercised both for rank-2 free modules with the standard basis generators and for a single non-trivial generator (x, y), covering the 'v in <v>' style of query.
|
Documentation preview for this PR (built with commit 552a915; changes) is ready! 🎉 |
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
1 participant
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.
Issue: Containment test "vec in G" returned True for vectors not in G when the base ring was a multivariate polynomial ring (and several other cases), because the existing fallback element constructor in Module_free_ambient performed no actual membership check.
This commit completes and extends the prior partial fix:
free_module.py: in Module_free_ambient.element_constructor, always call _check_element_membership (not only over exact coordinate rings). Inexact rings such as CC[x,y] previously skipped the check entirely, leading to the original Incorrect containment test for submodule over multivariable polynomial ring #40301 bug for the zero submodule.
free_module.py: guard is_submodule against infinite recursion when self.ambient_module() is self, which happens for QuotientModule instances and caused stack overflows for subquotients.
submodule.py: rework Submodule_free_ambient._groebner_basis_contains so that
submodule.py: override contains in Submodule_free_ambient to use the membership check directly instead of relying on cross-parent equality (no coercion is registered between a submodule of a quotient module and the quotient itself).
Added regression doctests covering the original bug example, the ZZ[x] case from the issue comments, the inexact-ring zero submodule case, and the quotient module subquotient case.
Fixes #40301.
📝 Checklist
⌛ Dependencies