Do not cache the non-existence of coerce/convert map too often, and do not pretend that there is a conversion where it doesn't make sense at all

[simon-king-jena]

Sorry for the long ticket title.

About the first part of the title:

sage: P.<x,y> = QQ[]
sage: P.is_coercion_cached(x)
False
sage: P.coerce_map_from(x)
sage: P.is_coercion_cached(x)
True

Hence, there is a reference to x in the coercion cache for P. OK, by #715 and friends, the reference is weak --- unless x does not allow weak references, in which case the reference will be strong, and x would not be collectable. Hence, a potential memory leak.

About the second part:

sage: ZZ.convert_map_from(1)
Conversion map:
  From: Set of Python objects of 
  To:   Integer Ring

or

sage: P.convert_map_from(x)
Traceback (most recent call last):
...
TypeError: Cannot convert sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular to sage.structure.parent.Parent

I think it is not good that the error occurs. The answer of ZZ.convert_map_from(1) seems strange, but apparently those things actually occur, namely conversions from the category of sequences.

CC: @nbruin @jpflori

Component: coercion

Keywords: coercion conversion object cache

Author: Simon King

Reviewer: Nils Bruin

Merged: sage-5.7.beta0

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

[simon-king-jena]

Description changed:

--- 
+++ 
@@ -28,4 +28,5 @@
 ...
 TypeError: Cannot convert sage.rings.polynomial.multi_polynomial_libsingular.MPolynomial_libsingular to sage.structure.parent.Parent
 ```
-In both cases, I believe the answer should be "None".
+
+I think it is not good that the error occurs. The answer of `ZZ.convert_map_from(1)` seems strange, but apparently those things actually occur, namely conversions from the category of sequences.

[nbruin]

comment:2

Are you sure

ZZ.convert_map_from(10)

should not raise an error? Shouldn't the argument be a parent? Something like:

    if not(isinstance(S,type) or isinstance(type(S),sage.structure.parent.Parent):
        raise TypeError('convert maps only exist from parents')

This has the potential for getting hairy if you want this to work:

sage: A=AdditiveGroups()
sage: A(ZZ)
Free commutative group on generators [1]

because then parent(ZZ) should be a Parent and it's not. However, this doesn't seem to be the way categories work anyway and it's probably wise to keep a strict separation between sets (parents) and categories. It actually makes me wonder if parent(ZZ) should even work.

I know this all flies in the face of duck typing, but so does the whole category framework. I'm afraid that a fully duck typed computer algebra system would about as bad as an untyped one.

[simon-king-jena]

comment:3

Replying to @nbruin:

Are you sure

ZZ.convert_map_from(10)

should not raise an error?

No, I am not sure.

Shouldn't the argument be a parent?

Or a type, because we want a coercion (not just a conversion) between Python ints and Sage integers.

This has the potential for getting hairy if you want this to work:

sage: A=AdditiveGroups()
sage: A(ZZ)
Free commutative group on generators [1]

That's totally separate. AdditiveGroups(), which doesn't exist yet, is a category and not a parent. It would be desirable, though, to have

sage: ZZ.category() # current behaviour
Category of euclidean domains
sage: AdditiveGroups()(ZZ).category()  # not implemented
Category of additive groups

because then parent(ZZ)

ZZ has no parent, it is a parent. Elements have a parent, but parents have a category.

It actually makes me wonder if parent(ZZ) should even work.

parent(X) currently returns the type of X, if X has no method X.parent(). This is in order to make int(1)+1 work. I don't think one should slow it down by distinguishing cases like "if X is a parent then parent(X) should raise an error, but if X is anything else then either return X.parent() or type(X)".

Anyway. I tried to introduce a shortcut for Parent.coerce_map_from, such that None is returned (without caching) if the argument is neither a CategoryObject nor a type. Similarly, I introduced a shortcut for Parent.convert_map_from, but had to be more relaxed, because apparently convert maps also need to exist from sequences (which are SageObject, but not CategoryObject).

Result: It nearly worked. There was one segfault in sage.rings.polynomial.infinite_polynomial_element (which I wrote) and one doctest error.

[simon-king-jena]

comment:4

Replying to @nbruin:

I know this all flies in the face of duck typing, but so does the whole category framework.

One could say that the category framework is the opposite of duck typing: In duck typing, you'd test whether certain methods are available for X, and conclude that X belongs to the category of ducks. In the category framework, you would at some point initialise X as an object of the category of ducks, and the category then provides X with methods like "quack()", "swim()", "fly()" --- or, if the category framework can not provide generic methods, it does require that these methods are implemented.

But I actually think that's a strength of the category framework. And note that after category initialisation of X, it would to some extent be possible to determine X.category() from the methods X provides.

[nbruin]

comment:5

ZZ has no parent, it is a parent. Elements have a parent, but parents have a category.

Ah yes, keeping that distinction strict is probably a good idea, even if it's
not always desirable from a strictly mathematical point of view:

sage: I=ZZ.ideal(3)
sage: I(6)
TypeError: 'Ideal_pid' object is not callable
sage: parent(I)
Monoid of ideals of Integer Ring
sage: category(I)
Category of ring ideals in Integer Ring

so an ideal is an element and not a parent (although mathematically it's also a
a non-unitary ring and at the very least a ZZ-module). What's that category
doing on I though? Do elements have a parent as well as a category?

sage: V=FreeModule(ZZ,1)
sage: W=V.span([3*V.0])
sage: parent(W)
<class 'sage.modules.free_module.FreeModule_submodule_pid_with_category'>
sage: category(W)
Category of modules with basis over Integer Ring

And here we're not getting a parent "... of submodules of V", whatever the ...
should be.

Along these lines, by the way:

sage: NumberField(x^2+1,name='i').ideal(3)
Fractional ideal (3)
sage: QQ.ideal(3)
Principal ideal (1) of Rational Field

illustrating the usual schism between algebraists and number theorists.

I don't think I'm really trying to make any point here. I'm just checking to
what degree Sage agrees with my mathematical intuition.

[simon-king-jena]

Attachment: trac_13378-convert_map_shortcut.patch.gz

Test validity of input before a (failing) construction of coercion

[simon-king-jena]

Author: Simon King

[simon-king-jena]

comment:6

I am not totally sure if the patch has dependencies, but let's test. With the patch, it is tested whether the input is valid as the domain of a conversion map or a coercion map. There is a difference between the two cases! Namely, Sequences are OK for a conversion map, but not for a coercion map.

If the input S is invalid, P.coerce_map_from(S) returns None, which is the expected answer if there is no coercion (hence, no error is raised). Because the short-cut is quick enough and in order to not pollute the cache, the answer is not cached.