structure/detect element action

[mantepse]

We fix a typo in the documentation and a bug in the implementation of structure.coerce_actions.detect_element_action.

Previously, we had

sage: detect_element_action(S3, R, False)
Right action by Symmetric group of order 3! as a permutation group on Multivariate Polynomial Ring in x, y, z over Rational Field

which is wrong, because detect_element_action should (despite its docstring) return an action of the second argument on the first.

Authors: @tscrim, @mantepse

[github-actions]

Documentation preview for this PR (built with commit 2f381d9; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[tscrim]

Interesting...The failure in categories/action.pyx is trivial; just need to swap the input order.

Now the failures in Lie algebras, that is because I didn't implement the _act_on_ correctly for the PBW basis elements. This change fixes those failures:

diff --git a/src/sage/algebras/lie_algebras/poincare_birkhoff_witt.py b/src/sage/algebras/lie_algebras/poincare_birkhoff_witt.py
index 3b59303d0d..bb808d1c2d 100644
--- a/src/sage/algebras/lie_algebras/poincare_birkhoff_witt.py
+++ b/src/sage/algebras/lie_algebras/poincare_birkhoff_witt.py
@@ -609,24 +609,24 @@ class PoincareBirkhoffWittBasis(CombinatorialFreeModule):
                 return ret
             cm = get_coercion_model()
             L = self.parent()._g
-            if self_on_left:
-                if cm.discover_action(L, x.parent(), mul):
-                    ret = x.parent().zero()
+            X = x.parent()
+            action = X.get_action(L, self_on_left=not self_on_left)
+            if action:
+                assert action.actor() is L
+                ret = X.zero()
+                if self_on_left:
                     for mon, coeff in self._monomial_coefficients.items():
                         term = coeff * x
                         for k, exp in reversed(mon._sorted_items()):
                             for _ in range(exp):
                                 term = L.monomial(k) * term
                         ret += term
-                    return ret
-            else:
-                if cm.discover_action(x.parent(), L, mul):
-                    ret = x.parent().zero()
+                else:
                     for mon, coeff in self._monomial_coefficients.items():
-                        term = coeff * x
+                        term = x * coeff
                         for k, exp in reversed(mon._sorted_items()):
                             for _ in range(exp):
                                 term = term * L.monomial(k)
                         ret += term
-                    return ret
+                return ret
             return None

While investigating this, I also noticed that the documentation of Parent.get_action() is also wrong!! It is only checking if there is an action of S on self!

[tscrim]

So the documentation for get_action() is not completely wrong as _get_action_ can return an action going either way, which is reasonable. However, beyond that, the discovery process is one sided.

For the argument_groups.py failure, I would say that is because the testing is only one sided. We probably should expand that to do what the documentation says (so it covers, e.g., int's). If we do this, then the assert in the diff above will need to become part of the if check.

I don't understand the change in output for the isometry group test. It is possibly related to the code not following the documentation of get_action().

There is a small possibility that this could fix the segfault noticed #37437, but I am very skeptical that it will.

[tscrim]

The src/sage/modules/free_quadratic_module_integer_symmetric.py failure should be the same as for matrix_gps/isometries.py.

[mantepse]

Shall I do the changes and push them here?

[tscrim]

Please do so. I still need to try and figure out the other failures/changes. @nbruin If you have any thoughts about this, that would be appreciated.

[mantepse]

Interesting...The failure in categories/action.pyx is trivial; just need to swap the input order.

Sorry, I don't know what you mean here.

[tscrim]

Interesting...The failure in categories/action.pyx is trivial; just need to swap the input order.

Sorry, I don't know what you mean here.

If you look at the failure and what the intended output should be, I think it is clear how to update the doctest (input).

[mantepse]

Interesting...The failure in categories/action.pyx is trivial; just need to swap the input order.

Sorry, I don't know what you mean here.

If you look at the failure and what the intended output should be, I think it is clear how to update the doctest (input).

I have, with this branch:

sage: cm = get_coercion_model()
sage: cm.get_action(ZZ, list, operator.mul)
sage: cm.get_action(list, ZZ, operator.mul)

that is, in both cases the output is None.

If I understand the doctring of cm.get_action correctly, it should be somewhat symmetric in R and S - although, for

sage: A = cm.get_action(QQx, ZZ, operator.truediv); A

it isn't.

[tscrim]

It looked trivial, but perhaps it is a bit more complicated than I thought. It might come down to the requirement that both X and Y are now required to be Parent subclasses, which we probably want to weaken. I will try to look at this soon.

[mantepse]

If we follow the docstring as defined by elements of X, this looks wrong. Indeed, in the old code, we were only looking at methods of x.

[tscrim]

I don't quite think it is saying that it defined by the methods of x (the action is defined by x using the method of y is how I read this), but you can read it that way. Although I don't think the docstring is accurate in this regard either.

[mantepse]

I 'traced' (using print)

sage: cm = get_coercion_model()
sage: c = cm.get_action(ZZ, list, operator.mul)

and it is line 230 that returns

Left action by Integer Ring on <class 'list'>

[tscrim]

Yea, I think we are just being too strict with making sure Y is a parent. We need to allow that to not be a Parent, and maybe need to catch an error (or fix some other code). Not 100% sure at this point.

[mantepse]

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?

[tscrim]

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?

[nbruin]

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 where Parent 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!)

[mantepse]

I just checked: the current branch does not fix #37976, but I think it should.

Using develop or this branch both give:

sage: h = SymmetricFunctions(QQ).h()
sage: v = vector([h[3]+h[2,1]])
sage: P = v.parent()
sage: from sage.structure.coerce_actions import detect_element_action
sage: a = detect_element_action(P, ZZ, True, v, -22)
sage: a
Right scalar multiplication by Integer Ring on Ambient free module of rank 1 over the integral domain Symmetric Functions over Rational Field in the homogeneous basis
sage: a.op
<built-in function mul>

Update: I'm afraid I misunderstood <built-in function mul> - this does call __mul__, right?

Update 2: I think that I now understand that #37976 has nothing to do with detect_element_action, sorry for the noise.