-
-
Notifications
You must be signed in to change notification settings - Fork 481
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
structure/detect element action #37918
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -159,7 +159,7 @@ cdef class ActedUponAction(GenericAction): | |
|
||
def detect_element_action(Parent X, Y, bint X_on_left, X_el=None, Y_el=None): | ||
r""" | ||
Return an action of X on Y as defined by elements of X, if any. | ||
Return an action of Y on X as defined by elements of X, if any. | ||
|
||
EXAMPLES: | ||
|
||
|
@@ -197,6 +197,23 @@ def detect_element_action(Parent X, Y, bint X_on_left, X_el=None, Y_el=None): | |
Traceback (most recent call last): | ||
... | ||
RuntimeError: an_element() for <__main__.MyParent object at ...> returned None | ||
|
||
Check that we have a right action of the symmetric group on the | ||
polynomial ring, but not a left action:: | ||
|
||
sage: S3 = SymmetricGroup(3) | ||
sage: R.<x,y,z> = QQ[] | ||
sage: detect_element_action(R, S3, True) | ||
Right action by Symmetric group of order 3! as a permutation group on | ||
Multivariate Polynomial Ring in x, y, z over Rational Field | ||
sage: detect_element_action(R, S3, False) | ||
|
||
Also, we don't have an action of the polynomial ring on the | ||
symmetric group:: | ||
|
||
sage: detect_element_action(S3, R, True) | ||
sage: detect_element_action(S3, R, False) | ||
|
||
""" | ||
cdef Element x | ||
|
||
|
@@ -224,15 +241,15 @@ def detect_element_action(Parent X, Y, bint X_on_left, X_el=None, Y_el=None): | |
except CoercionException as msg: | ||
_record_exception() | ||
|
||
# element x defining _act_on_ | ||
try: | ||
if x._act_on_(y, X_on_left) is not None: | ||
return ActOnAction(X, Y, X_on_left, False) | ||
except CoercionException: | ||
_record_exception() | ||
|
||
# element x defining _acted_upon_ | ||
if isinstance(Y, Parent): | ||
# element y defining _act_on_ | ||
try: | ||
if y._act_on_(x, not X_on_left) is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we follow the docstring There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't quite think it is saying that it defined by the methods of |
||
return ActOnAction(Y, X, not X_on_left, False) | ||
except CoercionException: | ||
_record_exception() | ||
|
||
# element x defining _acted_upon_ | ||
try: | ||
if x._acted_upon_(y, X_on_left) is not None: | ||
return ActedUponAction(Y, X, not X_on_left, False) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I 'traced' (using
print
)and it is line 230 that returns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I think we are just being too strict with making sure
Y
is a parent. We need to allow that to not be aParent
, and maybe need to catch an error (or fix some other code). Not 100% sure at this point.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, to be of any help I would need to know precisely what
detect_element_action
is supposed to be doing. And, I guess, that's part of the problem, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, we are kind of guessing at its purpose based on its code, its doc, and how it is used. Unfortunately there doesn’t seem to be any consistency with these three. @nbruin Do you have any recollections, thoughts, or opinions on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure which three routines are in play here. I don't think we can have an action of integers on a list, if that's what you're aiming for. Lists are way to coarse a "parent" to determine actions for. There may also be the problem that non-parents probably fail to have the right slots/attributes to properly participate in the coercion system. And there's the conflict that
3*[1,2,3]
and[1,2,3]*3
already means "repeated concatenation" in python.There was recently a discussion on action discovery concerning
int
vs.Integer
whereParent
participating in actions also came up: https://groups.google.com/g/sage-devel/c/RvmljIs7wBI/and in a different form in this relatively recent PR: #37369
(which in the end did end up with a patch so that action discovery for int on ZZ-modules should also work!)