cartesian product of algebra has troubles with base_ring

[dkrenn]

sage: cartesian_product((QQ['z'],))
Traceback (most recent call last)
...
/local/dakrenn/sage/6.9.beta6/local/lib/python2.7/site-packages/sage/categories/unital_algebras.pyc in __init_extra__(self)
--> 107             H = Hom(base_ring, self, Rings()) # TODO: non associative ring!
...
AttributeError: 'NoneType' object has no attribute '_is_category_initialized'

[dkrenn]

--- a/src/sage/structure/category_object.pyx
+++ b/src/sage/structure/category_object.pyx
@@ -623,10 +623,6 @@ cdef class CategoryObject(sage_object.SageObject):
             category) so as not to pollute the namespace of all
             category objects.
         """
+        try:
+            return super(CategoryObject, self).base_ring()
+        except AttributeError:
+            pass
         return self._base

solves this bug, but then

File "src/sage/categories/homset.py", line 596, in sage.categories.homset.Homset.__init__
Failed example:
    MyHomset(ZZ^3, ZZ^3, base = QQ).base_ring()
Expected:
    Rational Field
Got:
    Integer Ring

File "src/sage/categories/modules.py", line 519, in sage.categories.modules.Modules.Homsets.ParentMethods.base_ring
Failed example:
    H.base_ring.__module__
Expected nothing
Got:
    'sage.categories.modules'

[pbruin]

This still happens in SageMath 8.0.beta12.

[pbruin]

There are several related problems here, and it seems best to solve them together:

• the error described in the ticket description
• after fixing this in a minimal way, the base ring of the Cartesian product in the example is not initialised
• after fixing this as well, coercion from the base ring is not implemented.

The attached branch fixes the above problems essentially by getting rid of the method __init_extra__ of UnitalAlgebras, which used the somewhat problematic register_coercion() mechanism, and implementing equivalent functionality in _coerce_map_from_() instead. Because this method can (and is) overridden by other _coerce_map_from_() methods, those need to be adapted. Depending on the implementation of coercion from the base ring, this is done either through super (resulting in a coercion map implemented via _lmul_(), as in the old UnitalAlgebras.__init_extra__()) or by returning True (resulting in a DefaultConvertMap).

Some occurrences of x.parent() were changed to parent(x) in cases where x can be a Python type. Also, there was a bidirectional coercion between free algebras and their "PBW bases" which caused doctest failures; the coercion from the free algebra to the PBW bases was not needed and has been removed. Finally, this allows the hack introduced in #11900 (an attribute _no_generic_basering_coercion) can be removed, and seems to solve #16492.

[tscrim]

Replying to pbruin:

There are several related problems here, and it seems best to solve them together:

• the error described in the ticket description
• after fixing this in a minimal way, the base ring of the Cartesian product in the example is not initialised
• after fixing this as well, coercion from the base ring is not implemented.

The attached branch fixes the above problems essentially by getting rid of the method __init_extra__ of UnitalAlgebras, which used the somewhat problematic register_coercion() mechanism, and implementing equivalent functionality in _coerce_map_from_() instead. Because this method can (and is) overridden by other _coerce_map_from_() methods, those need to be adapted. Depending on the implementation of coercion from the base ring, this is done either through super (resulting in a coercion map implemented via _lmul_(), as in the old UnitalAlgebras.__init_extra__()) or by returning True (resulting in a DefaultConvertMap).

-1 to getting rid of this mechanism. It means you can no longer simply inherit from, e.g., CombinatorialFreeModule and set the category when creating a new (unital) algebra. It also acts as a safeguard to do something that every unital algebra should do.

Perhaps an improvement would be for __init_extra__ to first check to see if the usual _coerce_map_from_ does result in a coercion from the base ring. At least this would fix the need for the _no_generic_basering_coercion in polynomial rings. I haven't checked to see if this will fix the problem in the ticket description.

Also, technically, an implementation of _lmul_ is not a coercion, but an action (maybe with a coercion into the domain). From what I remember, I don't see a way this can be done within the coercion framework, which first tries to make both elements into the same parent and then do multiplication within that parent.

Some occurrences of x.parent() were changed to parent(x) in cases where x can be a Python type.

I've been told that this is faster too.

Also, there was a bidirectional coercion between free algebras and their "PBW bases" which caused doctest failures; the coercion from the free algebra to the PBW bases was not needed and has been removed.

This is an immediate -1 from me because they are the same object (in the mathematical sense), just expressed in terms of different bases. So there should, and needs, to be a bidirectional coercion. The fact that this needs to be removed is a very strong indication to me that this is not the correct fix.

Finally, this allows the hack introduced in #11900 (an attribute _no_generic_basering_coercion) can be removed, and seems to solve #16492.

However, the fact that _coerce_map_from_ returns True is a definite bug as everything (supposedly) coerces in. I do not believe it is something that _element_constructor_ can dictate.

[pbruin]

Replying to tscrim:

Replying to pbruin:

There are several related problems here, and it seems best to solve them together:

• the error described in the ticket description
• after fixing this in a minimal way, the base ring of the Cartesian product in the example is not initialised
• after fixing this as well, coercion from the base ring is not implemented.

The attached branch fixes the above problems essentially by getting rid of the method __init_extra__ of UnitalAlgebras, which used the somewhat problematic register_coercion() mechanism, and implementing equivalent functionality in _coerce_map_from_() instead. Because this method can (and is) overridden by other _coerce_map_from_() methods, those need to be adapted. Depending on the implementation of coercion from the base ring, this is done either through super (resulting in a coercion map implemented via _lmul_(), as in the old UnitalAlgebras.__init_extra__()) or by returning True (resulting in a DefaultConvertMap).

-1 to getting rid of this mechanism. It means you can no longer simply inherit from, e.g., CombinatorialFreeModule and set the category when creating a new (unital) algebra.

This is not true; if you do this, then everything should work in the same way as before except the default coercion map is now discovered by _coerce_map_from_(). Of course, if you override _coerce_map_from_() then you either have to make sure you return a custom coercion map from the base ring (if you have one), or call super to get the default one, implemented via _lmul_(). I am convinced that this is a feature, not a bug, and that the existing method has several disadvantages:

• because the coercion map is constructed in __init_extra__(), it is always constructed, even if you are never going to use it;
• as a consequence, if a user wants to declare a coercion map to or from an algebra, this is impossible because the coercion system has already been invoked;
• similarly, when writing a new class of algebras (in my case I was trying to build one on top of Cartesian products of algebras), and you want to use your own coercion from the base ring, you have to resort to the ugly _no_generic_basering_coercion hack.

[pbruin]

Replying to tscrim:

Perhaps an improvement would be for __init_extra__ to first check to see if the usual _coerce_map_from_ does result in a coercion from the base ring. At least this would fix the need for the _no_generic_basering_coercion in polynomial rings. I haven't checked to see if this will fix the problem in the ticket description.

By definition, when __init_extra__ runs the parent hasn't been fully initialised yet, so this sounds fragile to me. My impression is that the problem that such a construction would solve is solved in a more standard and flexible way by using super in _coerce_map_from_() if necessary.

[pbruin]

Replying to tscrim:

Also, technically, an implementation of _lmul_ is not a coercion, but an action (maybe with a coercion into the domain). From what I remember, I don't see a way this can be done within the coercion framework, which first tries to make both elements into the same parent and then do multiplication within that parent.

If you look at the code of UnitalAlgebras.ParentMethods.__init_extra__(), you will see that the default coercion map from the base ring is implemented as an action via _lmul_(). The coercion framework does discover and use actions, not just coercion into a common parent. For example:

sage: cm = get_coercion_model()
sage: R.<x> = QQ[]
sage: cm.explain(1, x)
Action discovered.
    Left scalar multiplication by Integer Ring on Univariate Polynomial Ring in x over Rational Field
Result lives in Univariate Polynomial Ring in x over Rational Field
Univariate Polynomial Ring in x over Rational Field

[pbruin]

Replying to tscrim:

Also, there was a bidirectional coercion between free algebras and their "PBW bases" which caused doctest failures; the coercion from the free algebra to the PBW bases was not needed and has been removed.

This is an immediate -1 from me because they are the same object (in the mathematical sense), just expressed in terms of different bases. So there should, and needs, to be a bidirectional coercion. The fact that this needs to be removed is a very strong indication to me that this is not the correct fix.

Bidirectional coercions should always be avoided, for at least two reasons:

• If X and Y are two parents with bidirectional coercion maps, and if x and y are elements of X and Y, respectively, then it is not clear where x + y should live.
• ​Bidirectional coercion implies memory leak.

[pbruin]

Replying to tscrim:

However, the fact that _coerce_map_from_ returns True is a definite bug as everything (supposedly) coerces in. I do not believe it is something that _element_constructor_ can dictate.

That is true; I was careless here and will fix that.

[tscrim]

Replying to pbruin:

Replying to tscrim:

Perhaps an improvement would be for __init_extra__ to first check to see if the usual _coerce_map_from_ does result in a coercion from the base ring. At least this would fix the need for the _no_generic_basering_coercion in polynomial rings. I haven't checked to see if this will fix the problem in the ticket description.

By definition, when __init_extra__ runs the parent hasn't been fully initialised yet, so this sounds fragile to me. My impression is that the problem that such a construction would solve is solved in a more standard and flexible way by using super in _coerce_map_from_() if necessary.

It is the last thing run by Parent.__init__, so everything is properly initialized (at least that matters for this). So it should be robust. It also more strongly tries to enforce that an object in this category has certain features.

I am also very strongly opposed to implementing coercions using super calls within categories. First, there are technical limitations. Second, it can be natural that you do not want to call _coerce_map_from_ going up the MRO because you have extra structure (e.g., you are not a full subcategory). For example, there may be no coercion as graded modules that would be there as modules. Yes, there are ways around that, but they are not as pretty or specific.

You also are likely to get a speed regression when finding if coercions exists because of the extra walk up through the category MRO.

[tscrim]

Replying to pbruin:

Replying to tscrim:

Also, technically, an implementation of _lmul_ is not a coercion, but an action (maybe with a coercion into the domain). From what I remember, I don't see a way this can be done within the coercion framework, which first tries to make both elements into the same parent and then do multiplication within that parent.

If you look at the code of UnitalAlgebras.ParentMethods.__init_extra__(), you will see that the default coercion map from the base ring is implemented as an action via _lmul_().

I just don't quite agree with the phrasing as it makes it seem like _lmul_ is the coercion, not wrapped in a SetMorphism. Although IIRC, this is moot because actions has precedence over coercions.

[tscrim]

Replying to pbruin:

Replying to tscrim:

Replying to pbruin:

There are several related problems here, and it seems best to solve them together:

• the error described in the ticket description
• after fixing this in a minimal way, the base ring of the Cartesian product in the example is not initialised
• after fixing this as well, coercion from the base ring is not implemented.

The attached branch fixes the above problems essentially by getting rid of the method __init_extra__ of UnitalAlgebras, which used the somewhat problematic register_coercion() mechanism, and implementing equivalent functionality in _coerce_map_from_() instead. Because this method can (and is) overridden by other _coerce_map_from_() methods, those need to be adapted. Depending on the implementation of coercion from the base ring, this is done either through super (resulting in a coercion map implemented via _lmul_(), as in the old UnitalAlgebras.__init_extra__()) or by returning True (resulting in a DefaultConvertMap).

-1 to getting rid of this mechanism. It means you can no longer simply inherit from, e.g., CombinatorialFreeModule and set the category when creating a new (unital) algebra.

This is not true; if you do this, then everything should work in the same way as before except the default coercion map is now discovered by _coerce_map_from_().

Ah, right. Sorry, I missed the change in module.pyx.

Of course, if you override _coerce_map_from_() then you either have to make sure you return a custom coercion map from the base ring (if you have one), or call super to get the default one, implemented via _lmul_(). I am convinced that this is a feature, not a bug,

I never claimed this was a bug. However, I am worried about speed regressions here from walking up the MRO, as well as enforcing it could lead to some mathematical issues because possible (natural) coercion is inversely proportional to the amount of structure. See my other reply.

and that the existing method has several disadvantages:

• because the coercion map is constructed in __init_extra__(), it is always constructed, even if you are never going to use it;

I see this as an enforcement of the category requirements. However, it does have a cost when constructing the parents, and I agree that it should be avoiding.

• as a consequence, if a user wants to declare a coercion map to or from an algebra, this is impossible because the coercion system has already been invoked;

No, that is not true. You are adding a coercion map. So you can always add additional coercions. With develop:

sage: C = CombinatorialFreeModule(QQ, ['a','b'])
sage: D = CombinatorialFreeModule(QQ, ['a','b','c'])
sage: phi = C.module_morphism(D.basis().__getitem__, codomain=D)
sage: C.register_embedding(phi)
sage: D(C.basis()['a']).parent()
Free module generated by {'a', 'b', 'c'} over Rational Field

It only freezes register_embedding when you invoke the coercion framework directly:

sage: psi = C.module_morphism(E.basis().__getitem__, codomain=E)
sage: psi.register_as_coercion()

• similarly, when writing a new class of algebras (in my case I was trying to build one on top of Cartesian products of algebras), and you want to use your own coercion from the base ring, you have to resort to the ugly _no_generic_basering_coercion hack.

In some ways that is a workaround, but it also gives you finer control.

[tscrim]

Replying to pbruin:

Replying to tscrim:

Also, there was a bidirectional coercion between free algebras and their "PBW bases" which caused doctest failures; the coercion from the free algebra to the PBW bases was not needed and has been removed.

This is an immediate -1 from me because they are the same object (in the mathematical sense), just expressed in terms of different bases. So there should, and needs, to be a bidirectional coercion. The fact that this needs to be removed is a very strong indication to me that this is not the correct fix.

Bidirectional coercions should always be avoided, for at least two reasons:

• If X and Y are two parents with bidirectional coercion maps, and if x and y are elements of X and Y, respectively, then it is not clear where x + y should live.

IMO, ambiguity is the correct thing. They are different representations of the same parent, and in general, why should one be preferred over the other? For PBW bases here, the natural monomial basis is "better" in some sense, but since you created the special PBW basis, you probably want the PBW. What about for things like symmetric functions or Iwahori-Hecke algebras that have several bases that all sit on equal footing?

• ​Bidirectional coercion implies memory leak.

Excuse a little theatrics, but my first thought was, "that's horrifying". I believe I understand why this happens, but I feel that is more of a problem of a bad caching mechanism in the coercion framework. It also puts a much heavier burden on someone writing code because you will have to avoid all coercion cycles.

My initial thought is also that you cannot get around this by specifying conversion maps because it puts the other parent as the key. I could see it making the non-coercion direction slower if the inverse map was implemented by inverting one a ModuleMorphismByLinearity, used caching of elements (this might be a memory leak in and of itself), or some other form of needing to create a morphism. It also would make the code of _element_constructor_ much more complicated with extra cases, especially when a mathematical object has a lot of realizations.

[pbruin]

I am now working on a more minimal solution. After splitting off part of it into #29225, I realised that time has progressed by exactly 10⁴ tickets...

[pbruin]

The approach in this new branch is to move the machinery for setting up the coercion from the base ring of an algebra into a separate method _coerce_map_from_base_ring(). This is called by the existing __init_extra__() (so it has the same effect as before), but also by a new default _coerce_map_from_() method. This then registers the coercion in cases where __init_extra__() runs at a time when not everything is initialised.

[tscrim]

This comment needs to be changed

-# We need to register a coercion from the base ring to self.
+# We need to construct a coercion from the base ring to self.

I think your _lmul_ can create elements in the parent that are not actually suppose to be in the parent. Specifically, _cartesian_product_of_elements does not do any safety checking (nor do I think it should as it is meant to be called internally with safe input). So in your example, if you instead multiplied by 1/5, I think you get an element of B that is not in B.

Do Jordan algebras still need the _no_generic_basering_coercion? Can we also get rid of it for matrix spaces and polynomial rings?

Also why should we allow for base_ring is None in __init_extra__ to be valid? I feel like if we get to that point and there is no base ring, then it is a bug (as we have declared the syntax of the base ring must be setup during the __init__ call) and it should result in an error.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​dcc25ea

Trac 19225: update comment

[pbruin]

Replying to tscrim:

I think your _lmul_ can create elements in the parent that are not actually suppose to be in the parent. Specifically, _cartesian_product_of_elements does not do any safety checking (nor do I think it should as it is meant to be called internally with safe input). So in your example, if you instead multiplied by 1/5, I think you get an element of B that is not in B.

It doesn't seem this happens, because _lmul_() itself is also called with safe input:

sage: A = FreeModule(ZZ, 2)
sage: B = cartesian_product([A, A])
sage: 1/5*B(([1, 2], [3, 4]))
Traceback (most recent call last):
...
TypeError: unsupported operand parent(s) for *: 'Rational Field' and 'The Cartesian product of (Ambient free module of rank 2 over the principal ideal domain Integer Ring, Ambient free module of rank 2 over the principal ideal domain Integer Ring)'

Do Jordan algebras still need the _no_generic_basering_coercion? Can we also get rid of it for matrix spaces and polynomial rings?

Yes, but I prefer to do this on a separate ticket. For Jordan algebras it is trivial, for matrix algebras it requires adding a one-line _coerce_map_from_base_ring() method, but for polynomial rings it is somewhat less trivial.

Also why should we allow for base_ring is None in __init_extra__ to be valid? I feel like if we get to that point and there is no base ring, then it is a bug (as we have declared the syntax of the base ring must be setup during the __init__ call) and it should result in an error.

The error reported in this ticket occurs inside an __init_extra__() method. The problem is that in cases where the base ring is initialised by the category (as it is for Cartesian products), it is also done in an __init_extra__() method, and in this case the __init_extra__() of unital algebras is called before the __init_extra__() of Cartesian products. The order in which the __init_extra__() methods of the various categories are called is determined by the method resolution order; since this is dynamically constructed by the category framework, I think it will be hard to come up with a robust way of always getting the order correct, so I think it is better to be flexible and have _coerce_map_from_() as a fallback, allowing us to retry registering the coercion after all __init_extra__() methods have run.

[tscrim]

Replying to pbruin:

Replying to tscrim:

I think your _lmul_ can create elements in the parent that are not actually suppose to be in the parent. Specifically, _cartesian_product_of_elements does not do any safety checking (nor do I think it should as it is meant to be called internally with safe input). So in your example, if you instead multiplied by 1/5, I think you get an element of B that is not in B.

It doesn't seem this happens, because _lmul_() itself is also called with safe input:

sage: A = FreeModule(ZZ, 2)
sage: B = cartesian_product([A, A])
sage: 1/5*B(([1, 2], [3, 4]))
Traceback (most recent call last):
...
TypeError: unsupported operand parent(s) for *: 'Rational Field' and 'The Cartesian product of (Ambient free module of rank 2 over the principal ideal domain Integer Ring, Ambient free module of rank 2 over the principal ideal domain Integer Ring)'

I guess I was misremembering, so it is a bit harder to define an action using _lmul_ (and _rmul_) than _acted_on_ (which does not check beforehand). I thought they had the same behavior in that regards. (Side note, now I am curious where the QQ action on A is defined.) Although I would argue there should be an action supported on B coming from the composition diagonal action of QQ on A. I guess that is essentially outside the scope of this ticket.

Also why should we allow for base_ring is None in __init_extra__ to be valid? I feel like if we get to that point and there is no base ring, then it is a bug (as we have declared the syntax of the base ring must be setup during the __init__ call) and it should result in an error.

The error reported in this ticket occurs inside an __init_extra__() method. The problem is that in cases where the base ring is initialised by the category (as it is for Cartesian products), it is also done in an __init_extra__() method, and in this case the __init_extra__() of unital algebras is called before the __init_extra__() of Cartesian products. The order in which the __init_extra__() methods of the various categories are called is determined by the method resolution order; since this is dynamically constructed by the category framework, I think it will be hard to come up with a robust way of always getting the order correct, so I think it is better to be flexible and have _coerce_map_from_() as a fallback, allowing us to retry registering the coercion after all __init_extra__() methods have run.

Ah, I see the issue. The categories of Cartesian products of modules is incomparable (in the poset category) with unital algebras. However, this is an argument for keeping a (cached) method base_ring that also sets _base in the category as base_ring() should be callable and initialized after __init__ has run. The other option that comes to my mind would be to explicitly call the UnitalAlgebras.CartesianProducts.__init_extra__ when base_ring is None as a way of stating we are forcing the ordering of the categories that should be; we would also make this the first if statement in that block. What do you think?

[pbruin]

Replying to tscrim:

Although I would argue there should be an action supported on B coming from the composition diagonal action of QQ on A. I guess that is essentially outside the scope of this ticket.

Yes, this has more to do with automatically constructing the "right" parent as in push-outs and functorial constructions.

Also why should we allow for base_ring is None in __init_extra__ to be valid? I feel like if we get to that point and there is no base ring, then it is a bug (as we have declared the syntax of the base ring must be setup during the __init__ call) and it should result in an error.

The error reported in this ticket occurs inside an __init_extra__() method. The problem is that in cases where the base ring is initialised by the category (as it is for Cartesian products), it is also done in an __init_extra__() method, and in this case the __init_extra__() of unital algebras is called before the __init_extra__() of Cartesian products. The order in which the __init_extra__() methods of the various categories are called is determined by the method resolution order; since this is dynamically constructed by the category framework, I think it will be hard to come up with a robust way of always getting the order correct, so I think it is better to be flexible and have _coerce_map_from_() as a fallback, allowing us to retry registering the coercion after all __init_extra__() methods have run.

Ah, I see the issue. The categories of Cartesian products of modules is incomparable (in the poset category) with unital algebras.

Yes, exactly.

However, this is an argument for keeping a (cached) method base_ring that also sets _base in the category as base_ring() should be callable and initialized after __init__ has run.

I don't think there is a problem here; the __init_extra__() methods are called as the last thing in Parent.__init__(), so after __init__() has finished, everything has been set up.

The other option that comes to my mind would be to explicitly call the UnitalAlgebras.CartesianProducts.__init_extra__ when base_ring is None as a way of stating we are forcing the ordering of the categories that should be; we would also make this the first if statement in that block. What do you think?

I don't like this very much; it sounds like an ad-hoc way of enforcing a certain order different from the method resolution order. Also, this would mean that UnitalAlgebras.CartesianProducts.__init_extra__() would be called twice.

[tscrim]

Replying to pbruin:

Replying to tscrim:

However, this is an argument for keeping a (cached) method base_ring that also sets _base in the category as base_ring() should be callable and initialized after __init__ has run.

I don't think there is a problem here; the __init_extra__() methods are called as the last thing in Parent.__init__(), so after __init__() has finished, everything has been set up.

I think I am convinced now that this is okay. Aince the coercion framework doesn't register a coercion from a parent P to itself, then there are no differences between defining the coercion between set with __init_extra__ and _coerce_map_from_. It feels like we should just outright remove the __init_extra__ now since it is functionally useless with the _coerce_map_from_. Although in order to do this, we have to require that all parents in the category always make a base call of _coerce_map_from_ if they have their own implementation. I think in general this is a good thing to do, but also maybe for a followup as this is a definite bug-fix.

I am not fully convinced that we should go through another super call of _coerce_map_from_, but since it should be called sparingly due to coercion caching, it shouldn't affect computation times. So if you are definitely okay with the current state, then you can set a positive review. Thank you for answering all my questions and addressing all my concerns.

---

Side note: A slight inconsistency I noticed

sage: ZZ.coerce_map_from(ZZ)
Identity endomorphism of Integer Ring
sage: coercion_model.discover_coercion(ZZ, ZZ)
(None, None)
sage: coercion_model.discover_coercion(ZZ, QQ)
((map internal to coercion system -- copy before use)
 Natural morphism:
   From: Integer Ring
   To:   Rational Field, None)
sage: QQ.coerce_map_from(ZZ)
Natural morphism:
  From: Integer Ring
  To:   Rational Field

but since discover_coercion is meant to be more internal-use only, this isn't anything that we need to fix IMO.

[pbruin]

Replying to tscrim:

Replying to pbruin:

Replying to tscrim:

However, this is an argument for keeping a (cached) method base_ring that also sets _base in the category as base_ring() should be callable and initialized after __init__ has run.

I don't think there is a problem here; the __init_extra__() methods are called as the last thing in Parent.__init__(), so after __init__() has finished, everything has been set up.

I think I am convinced now that this is okay. Aince the coercion framework doesn't register a coercion from a parent P to itself, then there are no differences between defining the coercion between set with __init_extra__ and _coerce_map_from_.

Indeed, it shouldn't matter when or how the coercion was registered.

It feels like we should just outright remove the __init_extra__ now since it is functionally useless with the _coerce_map_from_. Although in order to do this, we have to require that all parents in the category always make a base call of _coerce_map_from_ if they have their own implementation.

This was actually more or less how the previous branch (from 2017) worked, but from your comments at the time it seemed that you were against this...

I am not fully convinced that we should go through another super call of _coerce_map_from_, but since it should be called sparingly due to coercion caching, it shouldn't affect computation times. So if you are definitely okay with the current state, then you can set a positive review. Thank you for answering all my questions and addressing all my concerns.

OK, thank you for the review!

[tscrim]

Replying to pbruin:

Replying to tscrim:

I think I am convinced now that this is okay. Aince the coercion framework doesn't register a coercion from a parent P to itself, then there are no differences between defining the coercion between set with __init_extra__ and _coerce_map_from_.

Indeed, it shouldn't matter when or how the coercion was registered.

It feels like we should just outright remove the __init_extra__ now since it is functionally useless with the _coerce_map_from_. Although in order to do this, we have to require that all parents in the category always make a base call of _coerce_map_from_ if they have their own implementation.

This was actually more or less how the previous branch (from 2017) worked, but from your comments at the time it seemed that you were against this...

I was because I didn't see the necessity of changing the coercion registration mechanism and I wanted it to be done at initialization. I am still a little hesitant about doing it because it feels like it gives a little less control (or at least automatically) with how objects register coercions (e.g., there could be a natural coercion between objects as modules but not as graded modules), but the implementation just has to be more specific with the super() calls. Thanks again for bearing with me.

[SimonKing]

Unfortunately I cannot infer from the comments what one has to do if one has used the "ugly _no_generic_basering_coercion hack" in the past.

Shall one simply remove that attribute? Or what else to implement?

It is really unfortunate that clicking on the branch of a closed ticket doesn't provide information on what the ticket changed (or at least clicking on the branch for THIS ticket and searching for the string "_no_generic_basering_coercion" doesn't show a result).

[SimonKing]

Of course I didn't empty the "commit" field on purpose.

[SimonKing]

Replying to SimonKing:

Of course I didn't empty the "commit" field on purpose.

... and when I tried to fill it with its old value, it didn't help. I infer that the commit field of a closed ticket is of no use.

[pbruin]

Replying to SimonKing:

Unfortunately I cannot infer from the comments what one has to do if one has used the "ugly _no_generic_basering_coercion hack" in the past.

Shall one simply remove that attribute? Or what else to implement?

You can try removing it. If that doesn't work, you can implement a _coerce_map_from_base_ring() method; see the branches attached to this ticket and to #29247 for some examples. I think it is possible to make _coerce_map_from_base_ring() return None in case you really don't want a coercion map.

It is really unfortunate that clicking on the branch of a closed ticket doesn't provide information on what the ticket changed (or at least clicking on the branch for THIS ticket and searching for the string "_no_generic_basering_coercion" doesn't show a result).

In this case, that happens because _no_generic_basering_coercion was kept in place in this ticket and was only removed in #29247.

[SimonKing]

Replying to pbruin:

Shall one simply remove that attribute? Or what else to implement?

You can try removing it. If that doesn't work, you can implement a _coerce_map_from_base_ring() method; see the branches attached to this ticket and to #29247 for some examples. I think it is possible to make _coerce_map_from_base_ring() return None in case you really don't want a coercion map.

Thank you!

As it turns out, removing the attribute did work in my application.