Skip to content
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

Should not allow coercion from polynomials to symbolic ring #13360

Open
tkluck opened this issue Aug 11, 2012 · 23 comments
Open

Should not allow coercion from polynomials to symbolic ring #13360

tkluck opened this issue Aug 11, 2012 · 23 comments

Comments

@tkluck
Copy link

tkluck commented Aug 11, 2012

I've been struggling with some unexpected results in my computations with polynomials over the symbolic ring. Eventually, I came to the conclusion that it should not be possible to coerce polynomials to the symbolic ring. (Note: I'm not saying that you can't convert to the symbolic ring!) Here's some reasons for disallowing it.

I've also attached an initial patch implementing this. It needs more work though, because the same thing holds for LaurentSeries and PowerSeries.

Reasons for not allowing PolynomialRing -> SR coercion:

  1. This inconsistency:
  sage: R.<t> = QQ[]
  sage: SR(t^2)
  t^2
  sage: R.<t> = SR[]
  sage: SR(t^2)
  Traceback (most recent call last):
  ...
  TypeError: not a constant polynomial

I think the first one should raise a TypeError, too.

  1. This one:
  sage: R.<t> = ZZ[]
  sage: (t + 1/2).parent()      # automatic base extension
  Univariate Polynomial Ring in t over Rational Field
  sage: R.<t> = ZZ[]
  sage: (t + pi).parent()   # expect base extension SR[t]
  Symbolic Ring

It is not hard to imagine that this gives all kinds of unexpected results when trying to manipulate these kind of rings programmatically. This must have cost me days and days.

  1. There is also a better way to convert a polynomial to the symbolic ring,
    namely just substituting a symbolic variable:
sage: R.<t> = PolynomialRing(ZZ,'t')
sage: f = t^2 - t
sage: t = var('t')
sage: f(t)
(t - 1)*t
sage: f(t).parent()
Symbolic Ring

This has the advantage that the conversion is explicit.

  1. Philosophically, I would say that the name of the variable is just a display label, but as it is, it has the side effect of determining what variable to coerce to. Then if you want to return a polynomial from a library routine, you can't be sure that it won't collide with a user-defined variable upon coercion.

Component: algebra

Work Issues: address review

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

@tkluck tkluck added this to the sage-5.11 milestone Aug 11, 2012
@tkluck
Copy link
Author

tkluck commented Aug 11, 2012

@tkluck

This comment has been minimized.

@nbruin
Copy link
Contributor

nbruin commented Aug 11, 2012

comment:3
  1. You probably agree that conversion from polynomial rings to SR should be possible. However, I'm afraid that requires coercion to easily work in the following example:
A.<a>=QQ[]
B.<b>=A[]
f=a^2*b+a*b^2
SR(f)

Even though there is an explicit conversion asked, I think the system uses coercion on the coefficients of f, which are elements of the polynomial ring A. The same thing applies even more essentially to evaluating f at a symbolic expression.

f(SR('x'))

It's always one level further down where coercions are really useful.

For your other examples:

1), 2) SR[]

That should really be an error already. Nothing useful will even come from defining polynomials over the symbolic ring.

[EDIT: As shown below, people DO find uses for them.]

SR is not a fully implemented ring. For instance, equality testing is spotty at best. It's not really an integral domain either:

max_symbolic(x,0)*min_symbolic(x,0)
  1. To some degree I agree with your philosophy, but Sage's coercion system doesn't. The variable names are part of the identity, e.q. QQ['x'] and QQ['x'] are identical objects, but QQ['x'] and QQ['y'] aren't (they aren't even equal). Similarly, elements from QQ['x'] coerce into QQ['x','y'] but not into QQ['a','b']. Coercing by name into SR is completely in step with that, whatever collisions that causes.
    It may be possible to override this behaviour by installing custom coercions.

@tkluck
Copy link
Author

tkluck commented Aug 12, 2012

comment:4

Replying to @nbruin:

  1. You probably agree that conversion from polynomial rings to SR should be possible.

Thanks for commenting, nbruin! Yes I agree (I actually mention that in italics right at the top).

However, I'm afraid that requires coercion to easily work in the following example:

A.<a>=QQ[]
B.<b>=A[]
f=a^2*b+a*b^2
SR(f)

I would suggest replacing this with

sage: a,b=var('a,b')
sage: f(a,b)

Which relates to ...

Even though there is an explicit conversion asked, I think the system uses coercion on the coefficients of f, which are elements of the polynomial ring A. The same thing applies even more essentially to evaluating f at a symbolic expression.

f(SR('x'))

I'm glad you mention this example. For me, this is not the same thing. In a way, it is exactly the distinction I'm trying to make with this ticket. I think your example should be allowed whereas coercion shouldn't.

For your other examples:

1), 2) SR[]

That should really be an error already. Nothing useful will even come from defining polynomials over the symbolic ring.

The sage documentation explicitly states that this is supported (at least in the case of the PowerSeriesRing).

As for it not being useful, I strongly disagree with that. As soon as you need coefficients like pi or sin(3), this is the only way to go. For example, to come back to your code:

sage: A.<a>=QQ[]
sage: B.<b>=A[]
sage: f=a^2*b+a*b^2
sage: f(b=pi)      # would expect an expansion in a, like pi*a^2 + pi^2*a
(pi*a = a^2)*pi
sage: f(b=pi).parent()   # I would expect: SR[a]
Symbolic Ring

Or, for a more specialized example: I'm doing calculations with Lax matrices in integrable systems. Then you want to calculate Laurent series in the spectral parameter, whereas all other variables are manipulated symbolically (i.e. functions can be applied to them). Just doing those calculations in the symbolic ring is much slower, you would need to collect and expand all the time, and the symbolic ring cannot keep track of big ohs.

Of course, this ticket is exactly about inconsistencies arising from polynomial rings over SR. If you believe those should raise an error already, than it is logical to say that that is the source of the inconsistency. If, like me, you believe that it is very useful, then you open a ticket like this one. :-)

  1. To some degree I agree with your philosophy, but Sage's coercion system doesn't. The variable names are part of the identity, e.q. QQ['x'] and QQ['x'] are identical objects, but QQ['x'] and QQ['y'] aren't (they aren't even equal). Similarly, elements from QQ['x'] coerce into QQ['x','y'] but not into QQ['a','b']. Coercing by name into SR is completely in step with that, whatever collisions that causes.

I think this is unfortunate, but I agree that the philosophy is in line with this.

@tkluck
Copy link
Author

tkluck commented Aug 12, 2012

comment:5
  1. To some degree I agree with your philosophy, but Sage's coercion system doesn't. The variable names are part of the identity, e.q. QQ['x'] and QQ['x'] are identical objects, but QQ['x'] and QQ['y'] aren't (they aren't even equal). Similarly, elements from QQ['x'] coerce into QQ['x','y'] but not into QQ['a','b']. Coercing by name into SR is completely in step with that, whatever collisions that causes.

I think this is unfortunate, but I agree that the philosophy is in line with this.

As an afterthought: the symbolic ring offers SR.symbol() which guarantees a unique variable that doesn't clash with user-defined ones. There's no such thing for polynomial rings. On the other hand, you could make your library code accept a variable name in its argument spec. Then it's the user's responsibility to prevent a name clash.

@nbruin
Copy link
Contributor

nbruin commented Aug 12, 2012

comment:6

OK, you seem to have some use cases where polynomials over SR do have some use. Indeed, basic arithmetic (but not division!) should be OK. I think what you need is that elements from polynomial rings over SR are not coercible into SR. A solution like this would need an expert in the coercion system to implement correctly, because the coercion system is one area of sage where a small and seemingly innocuous change can easily have huge consequences (for performance, correctness or both) for the entire system.

Roughly: if R.coerce_map_from(SR) is not None then SR.coerce_map_from(R['a',...]) should be None. I think that would remove most of the unexpected results.

@novoselt
Copy link
Member

comment:7

Polynomials with coefficients in SR are very useful and it is a pity that their support is not ideal - hopefully it will improve.

I do not agree that construction in 1) should give errors: as it is written, it is explicit conversion of a polynomial to the symbolic ring and I was annoyed by TypeError: not a constant polynomial recently, having to write SR(str(f)).

@nbruin
Copy link
Contributor

nbruin commented Aug 12, 2012

comment:8

Replying to @novoselt:

I do not agree that construction in 1) should give errors: as it is written, it is explicit conversion of a polynomial to the symbolic ring and I was annoyed by TypeError: not a constant polynomial recently, having to write SR(str(f)).

How about

R=SR['t']
f=SR('t') * R.0

It's much better if SR(f) gives an error for that. The error you're seeing is the failure of a non-constant polynomial to be converted to an element of its base ring, as expected. Really, what you're looking for is a "specialization" of your polynomial, effected by an evaluation:

f(SR('t'))

No reason to convert to strings.

Polynomial rings should really not be coercible to their base rings, and probably also not generally convertible to them (apart from constants). Sage's concept of coercion really relies on there not being "loops" in the coercion graph. They wreak havoc with finding common covering structures.

@novoselt
Copy link
Member

comment:9

I think it is completely unambiguous what do I mean by SR(f) if f is a polynomial - I want a symbolic expression equal to this polynomial with the same names for variables. It is an explicit call to perform a conversion, not a coercion. If it is necessary to handle this conversion separately, it should be done internally, transparent for the user.

f(SR('t')) is unintuitive and what am I supposed to do if I have a hundred variables in my polynomial ring? Cook up some dictionary substitution for evaluation which will be even less intuitive? I don't see how it would be better than SR(str(f)), and the best form would be SR(f).

I understand that loopy rings may cause difficulties in the framework, yet it is not a reason to cripple the symbolic ring. I think that pretty much anything has to be convertible to it. A possible solution for the situation in question may be "symbolic ring with black-listed names", so that if I want to consider a polynomial ring in t with symbolic coefficients, I explicitly create "SR that will never use t". Or perhaps a white-list version "SR that can only use {a, d, beta} for variable names" is better, especially if one creates iterated polynomial rings. Then there will be no collision between base ring and SR and clearly all such restricted rings are canonically coercible to SR, but not back. Note that such rings will also allow unambiguous conversion to polynomial rings with symbolic coefficients.

@tkluck
Copy link
Author

tkluck commented Aug 12, 2012

comment:10

Replying to @novoselt:

I think it is completely unambiguous what do I mean by SR(f) if f is a polynomial - I want a symbolic expression equal to this polynomial with the same names for variables. It is an explicit call to perform a conversion, not a coercion. If it is necessary to handle this conversion separately, it should be done internally, transparent for the user.

Even though I wrote this patch, I'm not too familiar with the coercion/conversion system. You may be right that it is easy to support SR(f) as an explicit conversion without coercions being possible. Because both coercion and conversion call the _symbolic_ method, my patch may be too zealous in removing this support.

For me, however, point 2) is so important that I would accept losing SR(f) if 2) cannot be handled otherwise. In that case, it would make sense to add a method to PolynomialRingElement that does the substitution behind the scenes.

I'll see what happens if I revert the modifications to _symbolic_ while retaining the ones to the coercion system.

@tkluck
Copy link
Author

tkluck commented Aug 12, 2012

comment:11

Replying to @nbruin:

Replying to @novoselt:

I do not agree that construction in 1) should give errors: as it is written, it is explicit conversion of a polynomial to the symbolic ring and I was annoyed by TypeError: not a constant polynomial recently, having to write SR(str(f)).

How about

R=SR['t']
f=SR('t') * R.0

It's much better if SR(f) gives an error for that.

This is actually a really good example. On second thought, I'm inclined to stand by my initial argument for removal of these conversions, too. I'm open to other views, though.

@tkluck
Copy link
Author

tkluck commented Aug 12, 2012

comment:12

Replying to @nbruin:

I think what you need is that elements from polynomial rings over SR are not coercible into SR.

From the code, I think that is exactly what's happening now. However, that is what leads to point 2). If an expert in the coercion system can make it such that t + pi coerces the summands into SR[t] instead of into SR, then maybe that would be a solution, too.

I'm sorry for spamming you guys by saying all this in three messages instead of one.

@tkluck
Copy link
Author

tkluck commented Aug 12, 2012

@nbruin
Copy link
Contributor

nbruin commented Aug 12, 2012

comment:13

Replying to @tkluck:

From the code, I think that is exactly what's happening now. However, that is what leads to point 2). If an expert in the coercion system can make it such that t + pi coerces the summands into SR[t] instead of into SR, then maybe that would be a solution, too.

I expect most people would find that undesirable. The most "economic" way of constructing t+pi if t=ZZ['t'].0 is to make the symbolic expression t+pi. It is not necessary to create the more complicated parent SR['t'] for this. Remember, polynomial rings over SR should be avoided for the most part. They are allowed to be constructed and have some limited use as you show, but sage should definitely not create them unless they are explicitly asked for.

@tkluck
Copy link
Author

tkluck commented Aug 12, 2012

comment:14

Replying to @nbruin:

Replying to @tkluck:
I expect most people would find that undesirable. The most "economic" way of constructing t+pi if t=ZZ['t'].0 is to make the symbolic expression t+pi. It is not necessary to create the more complicated parent SR['t'] for this. Remember, polynomial rings over SR should be avoided for the most part. They are allowed to be constructed and have some limited use as you show, but sage should definitely not create them unless they are explicitly asked for.

Are you sure? How about this example:

sage: R.<t> = ZZ[]
sage: (t + QQbar(sqrt(2))).parent()
Univariate Polynomial Ring in t over Algebraic Field
sage: (t + sqrt(2)).parent()
Symbolic Ring

Are you referring to any specific policy when saying "remember, polynomial rings over SR should be avoided"? Because I think they don't have to be and I'm trying to make their use as unsurprising as possible.

@nbruin
Copy link
Contributor

nbruin commented Aug 13, 2012

comment:15

Replying to @tkluck:

Are you sure? How about this example:

sage: R.<t> = ZZ[]
sage: (t + QQbar(sqrt(2))).parent()
Univariate Polynomial Ring in t over Algebraic Field

Exactly. t is transcendental over ZZ and hence over QQ, and QQbar contains no transcendentals. So it's clear an extension is needed.

sage: (t + sqrt(2)).parent()
Symbolic Ring

There is a transcendental in SR with the print name t, so that is a natural image for t the polynomial variable over ZZ. So both t and sqrt(2) fit in SR, so that's where the answer lives.

For your application, it seems that you would like to restrict SR to what one would consider constants in calculus, such as sin(2) etc. A kind of transcendental version of QQbar. The problem is that SR is not such a ring. It also contains all kinds of non-constant transcendentals. In fact, it would normally appear that SR contains all constructible transcendentals (for the right meaning of constructible).

That's where SR['T'] is special: You are clearly asking for a transcendental that doesn't lie in SR.
As you remark, sage already (justly) recognizes that by not allowing coercion of T into SR. So in my opinion sage already has exactly the right support for such rings.

@tkluck
Copy link
Author

tkluck commented Aug 13, 2012

comment:16

Replying to @nbruin:

Exactly. t is transcendental over ZZ and hence over QQ, and QQbar contains no transcendentals. So it's clear an extension is needed.

sage: (t + sqrt(2)).parent()
Symbolic Ring

There is a transcendental in SR with the print name t, so that is a natural image for t the polynomial variable over ZZ. So both t and sqrt(2) fit in SR, so that's where the answer lives.

This is a nice way of viewing it, and I cannot say that you are totally wrong. Still, someone who actually wants this can easily easily convert to the SR (by substitution or by me introducing a dedicated method), whereas someone who wants the behaviour I'm suggesting has no way of making that happen. In this sense, the current behaviour makes Sage less flexible than it needs to be, with the benefit of making it slightly less surprising for a user doing smaller on the command line.

For that user, however, it really makes no big difference whether the result is in SR or in SR[t]: it is just as possible to plot, evaluate, etc, elements of SR[t] w.r.t. to t as it is for elements of SR. I will concede that there may come a point when it will surprise them, though.

For your application, it seems that you would like to restrict SR to what one would consider constants in calculus, such as sin(2) etc. A kind of transcendental version of QQbar.

No, definitely not, that's just the simple examples I've been giving. In my use case, the coefficients are all functions of symbolic variables.

The problem is that SR is not such a ring. It also contains all kinds of non-constant transcendentals.

I'm not sure what difference that makes from the point of view of adding a transcendental not in SR.

That's where SR['T'] is special: You are clearly asking for a transcendental that doesn't lie in SR.
As you remark, sage already (justly) recognizes that by not allowing coercion of T into SR. So in my opinion sage already has exactly the right support for such rings.

Let me just give one more example. ZZ is an initial object for unitary rings, so this function:

def transformation(polynomial):
    R.<t> = ZZ[]
    return sum ( polynomial[i] * t^i for i in range(polynomial.degree())    

will be the identity for all polynomials over any base ring, except for the symbolic ring. This map may not seem useful in practise, but imagine applying some map to the coefficients, or anything more interesting. The point is that the above may be a special case of some complicated transformation you're doing on polynomials.

I know that one could probably work around this by

    R.<t> = polynomial.base_ring()[]

instead, but since we're talking about the coercion system, I'm trying to show examples where I think it doesn't work as expected. I'm also thinking of the case where a library algorithm I didn't write would work excellently on SR[] based on the above assumption, except for the nonstandard handling of Sage of SR as a base ring. Then it is a shame if Sage's coercion system would force you to dig into the algorithm because it would have required the programmer to continuously check for the anomalous condition 'base_ring == SR'.

@novoselt
Copy link
Member

comment:17

Well, it is about as anomalous as the set of all sets: it is the only ring with canonical coersions into it from arbitrarily construction that uses it as a base ring.

I am really starting to think that my previous idea of introducing white/black-listed symbolic rings would be a good solution and it should not be too hard to implement (although I don't know much about how SR is done).

SR is similar to automatic_names(True) - any name appearing in a string fed to it will be treated as a new variable. If I could write something like

sage: R = SR(allowed_names="a b beta")
sage: R("sqrt(a) * sin(beta)")
sqrt(a) * sin(beta)
sage: R("sqrt(a) * cos(alpha)")
...
NameError: alpha is not a variable in this symbolic ring

it would definitely suit my purposes and I think it would allow adjusting coercion in such a way, that treating polynomials in t over this R or adding a polynomial over integers and elements of R behaves consistently with regular rings.

By the way, I think that this is a bug:

sage: QQ["t"]["t"]
Univariate Polynomial Ring in t over Univariate Polynomial Ring in t over Rational Field

Nobody in mathematics uses the same variable in the same expression with the same meaning, so there is no reason to support it. Moreover, this may indicate a logical error and a user would appreciate catching it. So I think that constructions that "add names" should check that they are absent in the base ring. This way it would be prohibited to create SR["t"], but all polynomials can remain coercible to SR.

I suppose adding such a check is also not extremely difficult, but care should be taken when constructing polynomials "for internal purposes". There should be some standard way to get from a ring a name that can work as a "new name". And it should raise an exception for SR.

I also think that for any element of any ring we should have R(str(f)) == f which is impossible to hope for if we do not insist on distinct names for generators.

@nbruin
Copy link
Contributor

nbruin commented Aug 13, 2012

comment:18

Replying to @tkluck:

Let me just give one more example. ZZ is an initial object for unitary rings, so this function:

def transformation(polynomial):
    R.<t> = ZZ[]
    return sum ( polynomial[i] * t^i for i in range(polynomial.degree())    

will be the identity for all polynomials over any base ring, except for the symbolic ring. This map may not seem useful in practise, but imagine applying some map to the coefficients, or anything more interesting. The point is that the above may be a special case of some complicated transformation you're doing on polynomials.

That example doesn't work because sage refuses in a lot of cases to construct a suitable parent. Try:

transformation(ZZ['a']['b']([1,2,3]))

If it did work, it would only work for base rings that don't have a generator named t in them
already. So for any of

Q['t','s']['a','b']
`QuadraticField(5,name='t')
PowerSeriesRing(QQ,name='t')['u','v']

this would give a different answer than the functorial one I think you're expecting.

The fact that sage attaches meaning to print names of variables really does have some profound effects.

I know that one could probably work around this by

    R.<t> = polynomial.base_ring()[]

Indeed. It's much better to be explicit than making the system guess where you want your answer to live.

I would probably do

    t=polynomial.parent().0
    return sum ( polynomial[i] * t^i for i in range(polynomial.degree())

in order to avoid introducing new (hard coded) names unnecessarily.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@jdemeyer
Copy link

jdemeyer commented Dec 6, 2013

comment:20

One problem with your solution 3) is that the symbolic polynomial is not given in expanded form. I'm wondering if there is a proper way to evaluate a polynomial at a symbolic argument and get the symbolic polynomial in expanded form. I don't want to use expand(), because maybe I only want to expand the polynomial, not the argument in case the argument is something complicated.

@jdemeyer

This comment has been minimized.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin removed this from the sage-6.1 milestone Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin added this to the sage-6.2 milestone Jan 30, 2014
@rwst
Copy link

rwst commented Apr 3, 2014

Work Issues: address review

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@rwst
Copy link

rwst commented Jun 5, 2014

comment:24

Replying to @novoselt:

By the way, I think that this is a bug:

sage: QQ["t"]["t"]
Univariate Polynomial Ring in t over Univariate Polynomial Ring in t over Rational Field

This is now #16447

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants