wrong inverse action when using ConstructionFunctor.coercion_reversed

[dkrenn]

sage: Q.<y> = SR.subring(no_variables=True)[[]]
sage: (y / 1).parent()
Univariate Polynomial Ring in x over Symbolic Ring

but the result should be

Univariate Polynomial Ring in x over Symbolic Constants Subring

since there is a coercion from the integer ring to the symbolc constants subring.

This is because the attrirute coercion_reversed of ConstructionFunctor is not taken into account.

Depends on #19259

Component: coercion

Author: Daniel Krenn

Branch/Commit: 867d6d3

Reviewer: Benjamin Hackl

Issue created by migration from https://trac.sagemath.org/ticket/19521

[dkrenn]

comment:1

BTW: For some reason it works (partially) for polynomial rings:

     sage: R.<x> = SR.subring(no_variables=True)[]
     sage: (x / 1).parent()
     Univariate Polynomial Ring in x over Symbolic Constants Subring

but

     sage: cm = sage.structure.element.get_coercion_model()
     sage: cm.explain(x, 1, operator.div)
     Action discovered.
        Right inverse action by Symbolic Ring on Univariate Polynomial Ring in x over Symbolic Constants Subring
        with precomposition on right by Conversion map:
          From: Integer Ring
          To:   Symbolic Ring
    Result lives in Univariate Polynomial Ring in x over Symbolic Ring
    Univariate Polynomial Ring in x over Symbolic Ring

At one point this should be investigated, but probably on a different ticket.

[dkrenn]

Author: Daniel Krenn

[dkrenn]

Dependencies: #19259

[dkrenn]

Branch: u/dkrenn/coerce/inverse-action

[dkrenn]

Commit: 5638459

[dkrenn]

comment:4

Branch should fix this; still need to run a make ptestlong; will do this soon...

---

Last 10 new commits:

fb2885c	subring in index.rst
eab9513	simplify a doctest
23352b2	change ValueError to TypeError to make everything work with SR as it should
2d8b7c4	typo in docstring
15ec40e	docstring of SR.subring
7938d95	module description of subring
852959a	rename only_constants --> no_variables
e4837e9	correct parent of result of an_element
6e5a098	Merge branch 'u/dkrenn/symbolic-subring' of trac.sagemath.org:sage into coerce/inverse-action
5638459	#19521: use coercion_reversed in InverseAction

[dkrenn]

comment:5

Replying to @dkrenn:

Branch should fix this; still need to run a make ptestlong; will do this soon...

Passed. So I set it to needs review.

[behackl]

Reviewer: Benjamin Hackl

[behackl]

Changed commit from 5638459 to 867d6d3

[behackl]

Changed branch from u/dkrenn/coerce/inverse-action to u/behackl/coercion/inverse-action

[behackl]

comment:6

Hi! I've merged the (positively reviewed) dependency into this branch and reviewed the code in this ticket.

I think I understand your fix as well as the strategy behind it, and I am willing to give this a positive_review (i.e. everything LGTM). However, I'd rather be sure:

In principle, all that happens is that you order the functors and parents from smallest to largest (in the sense of coercion) and iterate over this reordered tower such that actually the smallest parent with the required property is found?

For example, (using your notation from the comment), if we had a tower A -> B <- C <- D -> E, then the old code would iterate over [A, B, C, D, E, F], and in your fixed version we iterate over [A, D, C, B, E]?

Also, I'd like to give the patchbots a chance to test this---or, at least, run make ptestlong myself.

---

New commits:

ff99c7f	Trac #19259: change to has_valid_variable
581f315	Trac #19259: check validity of variables
c9b2428	improve language
05dc834	misc. changes, indentation, line breaks
8cc884a	fix merge of functors
aeae8f3	Merge tag '7.0' into symbolics/symbolic-subring
c4a0e22	merge accepting and rejecting functors in all cases
d376b10	revert changes to merge of functors
867d6d3	Merge branch 'u/behackl/symbolics/symbolic-subring' of git://trac.sagemath.org/sage into coercion/inverse-action

[dkrenn]

comment:7

Replying to @behackl:

Hi! I've merged the (positively reviewed) dependency into this branch and reviewed the code in this ticket.

Thanks.

In principle, all that happens is that you order the functors and parents from smallest to largest (in the sense of coercion) and iterate over this reordered tower such that actually the smallest parent with the required property is found?

For example, (using your notation from the comment), if we had a tower A -> B <- C <- D -> E, then the old code would iterate over [A, B, C, D, E, F], and in your fixed version we iterate over [A, D, C, B, E]?

Correct.

Also, I'd like to give the patchbots a chance to test this---or, at least, run make ptestlong myself.

Ok.

[behackl]

comment:8

... alright. Passes ptestlong and does what it should -> positive_review.

[vbraun]

Changed branch from u/behackl/coercion/inverse-action to 867d6d3