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

New PR for action of MatrixGroup on a MPolynomialRing #35611

Open
wants to merge 37 commits into
base: develop
Choose a base branch
from
Open
Changes from 27 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
20d30f1
accounted for moved MatrixGroupElement_gap class
Bruno-TT May 4, 2023
3f56030
changed explicit type matching to isInstance
Bruno-TT May 4, 2023
61c0aa9
improved testing + correct polynomial var access
Bruno-TT May 4, 2023
bfd6f4a
variable naming conventions
Bruno-TT May 4, 2023
0e210d8
pycodestyle fix
Bruno-TT May 6, 2023
bd83e57
start of @ coersion implementation
Bruno-TT Jun 30, 2023
96b8446
adjusted _get_action_ signature
Bruno-TT Jul 4, 2023
e88c35f
pxd changes
Bruno-TT Jul 4, 2023
7d0a240
got coersion working
Bruno-TT Jul 6, 2023
4608cb6
Merge branch 'develop' into matrixgroup_polynomialring_action_new
Bruno-TT Jul 6, 2023
65ba9fd
linter fixes
Bruno-TT Jul 8, 2023
2ff7643
Merge branch 'develop' into matrixgroup_polynomialring_action_new
Bruno-TT Aug 23, 2023
aa0d722
remove space
Bruno-TT Aug 24, 2023
c746e0e
remove inherited pxd declaration
Bruno-TT Aug 24, 2023
cc2fea5
remove unneeded imports
Bruno-TT Aug 24, 2023
9d30130
avoid recomputing values
Bruno-TT Aug 25, 2023
162e248
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Aug 30, 2023
bc91221
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Aug 30, 2023
01e9de3
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Aug 30, 2023
1ca6a20
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Aug 30, 2023
2398542
Merge branch 'develop' into matrixgroup_polynomialring_action_new
Bruno-TT Aug 30, 2023
e3d1801
action object TestSuite implementation start
Bruno-TT Sep 2, 2023
8e858c5
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Oct 25, 2023
a2c1a8a
(broken) matpolyaction reduce method
Bruno-TT Oct 25, 2023
44333cb
Merge branch 'develop' into matrixgroup_polynomialring_action_new
Bruno-TT Nov 12, 2023
7892e72
test suite fixes (equality and hashing)
Bruno-TT Nov 12, 2023
6a5779f
doctest indentation fix
Bruno-TT Nov 12, 2023
59401d7
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Nov 13, 2023
a30b0bc
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Nov 13, 2023
864ede7
improve docstrings and doctests
Bruno-TT Nov 13, 2023
5e88a54
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Nov 19, 2023
b1ccf18
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Nov 19, 2023
95cfadc
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Nov 19, 2023
4ea3533
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Nov 19, 2023
cfd7b20
Update src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Bruno-TT Nov 19, 2023
5f4dbca
Merge branch 'develop' into matrixgroup_polynomialring_action_new
fchapoton Jan 18, 2024
468bb60
align the doc correctly
fchapoton Jan 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 76 additions & 1 deletion src/sage/rings/polynomial/multi_polynomial_ring_base.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ from sage.arith.misc import binomial

from sage.rings.integer_ring import ZZ

from . import polynomial_ring
from sage.categories.action import Action
from sage.matrix.matrix_space import MatrixSpace
from sage.matrix.constructor import matrix as MatrixConstructor
from operator import matmul

from . import multi_polynomial_ideal, polynomial_ring
from .term_order import TermOrder
from .polynomial_ring_constructor import (PolynomialRing,
polynomial_default_category)
Expand Down Expand Up @@ -105,6 +110,38 @@ cdef class MPolynomialRing_base(sage.rings.ring.CommutativeRing):
"""
return self.base_ring().is_integral_domain(proof)

cpdef _get_action_(self, G, op, bint self_on_left):
r"""
Return the action of ``G`` by ``op`` on ``self``.

EXAMPLES::

sage: G = groups.matrix.Sp(4,GF(2))
sage: R.<w,x,y,z>=GF(2)[]
sage: p=x*y^2 + w*x*y*z + 4*w^2*z+2*y*w^2
sage: g=G.1
sage: g
[0 0 1 0]
[1 0 0 0]
[0 0 0 1]
[0 1 0 0]
sage: g@p
w*x*y*z + w*z^2
sage: p2=x+y^2
sage: g2=G.0
sage: g2
[1 0 1 1]
[1 0 0 1]
[0 1 0 1]
[1 1 1 1]
sage: g2@p2
x^2 + z^2 + w + z
"""
from sage.groups.matrix_gps.matrix_group import MatrixGroup_generic
if isinstance(G, MatrixGroup_generic) and self.base_ring().has_coerce_map_from(G.base_ring()) and op == matmul and not self_on_left:
return MatrixPolynomialAction(G, self)
return super(MPolynomialRing_base, self)._get_action_(G, op, self_on_left)

def is_noetherian(self):
"""
EXAMPLES::
Expand Down Expand Up @@ -1794,3 +1831,41 @@ def unpickle_MPolynomialRing_generic(base_ring, n, names, order):
from .polynomial_ring_constructor import PolynomialRing

return PolynomialRing(base_ring, n, names=names, order=order)

class MatrixPolynomialAction(Action):
tscrim marked this conversation as resolved.
Show resolved Hide resolved
def __init__(self, MS, PR):
"""
Initialize ``self``.

EXAMPLES ::
sage: G = groups.matrix.Sp(4,GF(2))
sage: R.<w,x,y,z>=GF(2)[]
Bruno-TT marked this conversation as resolved.
Show resolved Hide resolved
sage: p=x*y^2 + w*x*y*z + 4*w^2*z+2*y*w^2
sage: g=G.1
sage: from operator import matmul
sage: A = p.parent()._get_action_(g.parent(), matmul, False)
sage: TestSuite(A).run()
"""
self._poly_vars = PR.gens()
self._vars_vector = MatrixConstructor(self._poly_vars).transpose()
self.MS=MS
self.PR=PR
Bruno-TT marked this conversation as resolved.
Show resolved Hide resolved
super().__init__(MS, PR, op=matmul)

def _act_(self, mat, polynomial):
assert mat.base_ring() == polynomial.base_ring()
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be better to check the base ring agreement in the __init__ and fail there. Although I think we only need a coercion from the matrices base ring into the polynomial base ring (e.g., matrices over ZZ have a natural action on polynomials over QQ).

Here, I think we should make sure the matrix is an element of self.MS and similarly for the polynomial. (Although that should already be the case by the time this method is called.)

This assert is also too strong as we only look for coercions of base rings, not equality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi,

I understand what you mean about moving the check to init, but some of the mathematical formalisms are a bit lost on me regarding the coercion check. I'm assuming I need to call has_coerce_map_from somehow, but I'm not sure what I need to call it on exactly. Any chance you could be a little more specific - my apologies for not understanding this better.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No problem; there is a lot going on with this.

Compare with the _get_action_(); you will see that you want polynomial.base_ring().has_coerce_map_from(mat.base_ring()) to be True. If you add the doctest I requested, you should see a failure once you do the action (unless I have missed something) before doing this change.

You can remove the assert afterwards, although if you really want a safety check (which really should not be needed at all) is mat.parent() is self.MS and polynomial.parent() is self.PR. I wouldn't add it though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok I've added

assert PR.has_coerce_map_from(MS)

to the start of __init__ and now it's throwing errors - assuming I've misunderstood somehow?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be the base rings:

assert PR.base_ring().has_coerce_map_from(MS.base_ring())

vars_to_sub_module_context = mat * self._vars_vector
vars_to_sub_ring_context = map(PolynomialRing(mat.base_ring(), self._poly_vars), vars_to_sub_module_context)
substitution_dict = {v: s for v, s in zip(self._poly_vars, vars_to_sub_ring_context)}
return polynomial.subs(substitution_dict)

def __eq__(self, other):
if isinstance(other, MatrixPolynomialAction):
return self.MS == other.MS and self.PR == other.PR
return False

def __hash__(self):
return hash((self.MS, self.PR))

def __reduce__(self):
return (type(self), (self.MS, self.PR))