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

derivative of integer wrt to variable in polynomial ring should belong to that ring, not symbolic ring #20812

Open
sagetrac-dgulotta mannequin opened this issue Jun 12, 2016 · 25 comments

Comments

@sagetrac-dgulotta
Copy link
Mannequin

sagetrac-dgulotta mannequin commented Jun 12, 2016

If I try to take the derivative of an integer (or nonzero rational, or integer mod n), then the result is an element of the symbolic ring:

sage: R.<x>=ZZ[]
sage: derivative(0,x).parent()
Symbolic Ring

It seems like it would be more natural for the returned value to belong to the ring containing x instead.

This may seem kind of pedantic, but it did trip me up when I was working with a list of polynomials, some of which were constant, and things were getting cast to Expression unexpectedly.

I am not particularly familiar with the Sage codebase, but I am attaching a patch that seems to fix the issue.

CC: @orlitzky

Component: calculus

Author: Daniel Gulotta, Vincent Delecroix

Branch/Commit: u/vdelecroix/20812 @ 04dbe4c

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

@sagetrac-dgulotta sagetrac-dgulotta mannequin added this to the sage-7.3 milestone Jun 12, 2016
@sagetrac-dgulotta
Copy link
Mannequin Author

sagetrac-dgulotta mannequin commented Jun 12, 2016

Attachment: functional.py.patch.gz

@fchapoton
Copy link
Contributor

comment:1

If you take care to use the correct zero, this just works:

sage: R.zero().derivative(x).parent()
Univariate Polynomial Ring in x over Integer Ring

But there is room for improvement, for sure.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 13, 2021

comment:2

The derivative of a symbolic expression is a symbolic expression :

sage: R1.<t>=ZZ[]
sage: derivative(x^2+x+1,t).parent()
Symbolic Ring

Therefore this :

sage: derivative(0,t).parent()
Symbolic Ring

is a special case, indicating that 0 is cast to a symbolic expression (probably by diff...).

However :

sage: derivative(R1(0),t).parent()
Univariate Polynomial Ring in t over Integer Ring

conforms to the requirement that the derivative of an object belongs its parent ring.

Pedantism works both ways...

==> marking as invalid and requesting review in order to close.

@EmmanuelCharpentier EmmanuelCharpentier mannequin removed this from the sage-7.3 milestone Mar 13, 2021
@sagetrac-dgulotta
Copy link
Mannequin Author

sagetrac-dgulotta mannequin commented Mar 13, 2021

comment:3

This behavior is confusing. I think it's reasonable to expect that the derivative(f,g) will lie in the smallest ring containing f and g. Why not fix this?

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 15, 2021

comment:4

Replying to @sagetrac-dgulotta:

This behavior is confusing. I think it's reasonable to expect that the derivative(f,g) will lie in the smallest ring containing f and g. Why not fix this?

In order

  • to avoid introducing a lot of special-casing...

  • to keep (at least an appearance of) reason : the differential of a "constant" does not make sense, whereas the differential of a function, expression or polynomial being respectively a function, expression or polynomial does...

...even when this function, expression or polynomial happens to be a "constant" or degree-0 monomial, in which case the derivative can be taken to be the null "constant" or degree-0 monomial.

Your remark may be more relevant in the reverse case:

sage: R1.<t>=QQbar[]
sage: foo=t^2
sage: integral(foo,t).parent()
Univariate Polynomial Ring in t over Algebraic Field

So far, so good. But

sage: integral(0,t).parent()
Symbolic Ring

is nonsensical, unless we mean to do implicitly :

sage: integral(R1(0),t).parent()
Univariate Polynomial Ring in t over Algebraic Field

In other words, take note that

sage: R1(0) is 0
False

even if

sage: R1(0).is_zero()
True

HTH,

@sagetrac-dgulotta
Copy link
Mannequin Author

sagetrac-dgulotta mannequin commented Mar 15, 2021

comment:5

It is difficult to do the right thing in all cases but I think that the patch that I submitted improves the situation for derivatives. I could write something similar for integrals if there is agreement that this would be useful.

The reason why I don't like casting things into the symbolic ring is that it leads to errors that are very difficult to track down. For example:

sage: R.<x>=QQ[]
sage: l = [1,x,x*(x-1),x*(x-1)*(x-2)]
sage: [derivative(f,x).monomial_coefficient(x) for f in l]
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-16-ff4d7a390725> in <module>
----> 1 [derivative(f,x).monomial_coefficient(x) for f in l]

<ipython-input-16-ff4d7a390725> in <listcomp>(.0)
----> 1 [derivative(f,x).monomial_coefficient(x) for f in l]

/ext/sage/sage-9.2/local/lib/python3.8/site-packages/sage/structure/element.pyx in sage.structure.element.Element.__getattr__ (build/cythonized/sage/structure/element.c:4703)()
    491             AttributeError: 'LeftZeroSemigroup_with_category.element_class' object has no attribute 'blah_blah'
    492         """
--> 493         return self.getattr_from_category(name)
    494 
    495     cdef getattr_from_category(self, name):
/ext/sage/sage-9.2/local/lib/python3.8/site-packages/sage/structure/element.pyx in sage.structure.element.Element.getattr_from_category (build/cythonized/sage/structure/element.c:4815)()
    504         else:
    505             cls = P._abstract_element_class
--> 506         return getattr_from_other_class(self, cls, name)
    507 
    508     def __dir__(self):
/ext/sage/sage-9.2/local/lib/python3.8/site-packages/sage/cpython/getattr.pyx in sage.cpython.getattr.getattr_from_other_class (build/cythonized/sage/cpython/getattr.c:2620)()
    370         dummy_error_message.cls = type(self)
    371         dummy_error_message.name = name
--> 372         raise AttributeError(dummy_error_message)
    373     attribute = <object>attr
    374     # Check for a descriptor (__get__ in Python)
AttributeError: 'sage.symbolic.expression.Expression' object has no attribute 'monomial_coefficient'

It is not at all clear from the error message that 1 needs to be replaced with R(1). And this is just a toy example; in real code the failure could occur much later down the line. So I think it is bad for a function like derivative that sometimes returns polynomials to implicitly cast things into the symbolic ring when none of the arguments live in the symbolic ring. The patch that I submitted should significantly reduce these types of errors.

In principle I think it is better to raise an error with a detailed message than to implicitly cast into the symbolic ring. I guess it may be too late to make that change since it might break existing code. Attempting to cast into a polynomial ring when possible seems like a reasonable compromise.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 15, 2021

comment:6

Replying to @sagetrac-dgulotta:

[ Snip... ]

The patch that I submitted should significantly reduce these types of errors.

Which patch ? I find no patch in the ticket.

More generally, I think that the problem is to define and compute "the smallest ring containing f and g".

To illustrate :

sage: R1.<t>=QQ[]
sage: t.derivative(t)
1
sage: t.derivative(t).parent()
Univariate Polynomial Ring in t over Rational Field
sage: 1.derivative(t).parent()
## [ Snip... ]
AttributeError: 'sage.rings.integer.Integer' object has no attribute 'derivative'
sage: t.parent()(1).derivative(t).parent()
Univariate Polynomial Ring in t over Rational Field

This cast is reasonable and might be expected (i. e. one can reasonably expect 1.differential(t) to return R1's 0.

Harder :

sage: R2.<u>=QQ[]
sage: t.derivative(u)
## [ Snip... ]
ValueError: cannot differentiate with respect to u

One might expect the 0 with :

  • this zero belonging to PolynomialRing(QQ,"v1,v2"), and
  • some "automagic glue" realizing v1==t, v2==u.

I'm not sure that this can be expressed in Sage...

For the integrals :

sage: 1.integral(t)
## [ Snip]
AttributeError: 'sage.rings.integer.Integer' object has no attribute 'integral'
sage: t.parent()(1).integral(t)
t

Again, a "reasonable" cast.

The case t.integral(u) leads to the same conclusion as for t.differentiate(u).

And we might have worse difficulties : what should be the "smallest ring" containing Zmod(3)['v'] and QQbar['w'] ? Ditto for ring of matrices...

Casting to SR is, indeed, far from ideal, but it seems tome that the possible enhancements are fraught with more difficulties than they solve.

Your thoughts ?

@sagetrac-dgulotta
Copy link
Mannequin Author

sagetrac-dgulotta mannequin commented Mar 15, 2021

comment:7

There is an attachment to this ticket, which is a patch. The patch uses sage.structure.element.get_coercion_model. I don't claim to be an expert on this function but it seems to do the right thing in cases that would come up in practice.

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Mar 15, 2021

comment:8

Replying to @sagetrac-dgulotta:

There is an attachment to this ticket, which is a patch. The patch uses sage.structure.element.get_coercion_model. I don't claim to be an expert on this function but it seems to do the right thing in cases that would come up in practice.

Would you mind submitting a branch, as described in Sagemath's developer's guide ?

@sagetrac-dgulotta
Copy link
Mannequin Author

sagetrac-dgulotta mannequin commented Mar 16, 2021

@videlec
Copy link
Contributor

videlec commented Mar 17, 2021

Commit: 0dc171f

@videlec
Copy link
Contributor

videlec commented Mar 17, 2021

comment:10

It is a bad idea to put a lot of code inside the try block. Only keep there the minimal amount of code that could potentially raise an error. For example neither elts.append(f) nor cm = get_coercion_model() should be there.


New commits:

0dc171fderivative: try to find a common parent before casting to SR

@tscrim
Copy link
Collaborator

tscrim commented Mar 19, 2021

comment:11

This might be an interesting data point:

sage: R.<x> = ZZ[]
sage: S.<y> = ZZ[]
sage: derivative(S.zero(), x).parent()
Univariate Polynomial Ring in y over Integer Ring

@videlec
Copy link
Contributor

videlec commented Dec 29, 2021

comment:12

Setting in needs works because of [comment:10].

@videlec
Copy link
Contributor

videlec commented Dec 29, 2021

comment:13

Also [comment:2] makes a good point derivative(QQ['t'].gen(), SR.var('t')) is currently a polynomial. The proposed branch would change that behaviour and it is not clear whether this is desirable.

A more sensible change for derivative(f, v) would be to do in order

  1. try to convert v to parent(f) and take derivative (that would keep the behaviour noticed in [comment:2])
  2. try to find a common parent for f and v and take derivative (that would solve the issue in the ticket description and arguably improve the behaviour from [comment:11])
  3. go to SR

@videlec videlec added this to the sage-9.6 milestone Dec 29, 2021
@videlec
Copy link
Contributor

videlec commented Dec 29, 2021

@videlec
Copy link
Contributor

videlec commented Dec 29, 2021

New commits:

3fd97b4derivative: try to find a common parent before casting to SR
04dbe4ccleaner implementation, doctest and explanations

@videlec
Copy link
Contributor

videlec commented Dec 29, 2021

Changed commit from 0dc171f to 04dbe4c

@videlec
Copy link
Contributor

videlec commented Dec 29, 2021

Author: Daniel Gulotta, Vincent Delecroix

@orlitzky
Copy link
Contributor

comment:17

There's a typo in,

This behaviour of the derivaive function

But personally, instead of an example that says

the parent of the result might seem confusing... this... is a consequence of how derivatives are implemented for polynomials

I would prefer if the documentation just told me what the parent will be in the OUTPUT block: the derivative of a symbolic expression with respect to a symbolic expression will be a symbolic expression, and likewise for polynomials with respect to polynomials. If the function and the variable are of different types that share a common parent, then the result will live in that common parent. Otherwise, as a last resort, it will live in the symbolic ring.

@videlec
Copy link
Contributor

videlec commented Dec 29, 2021

comment:18

Currently

sage: derivative(0)
0
sage: derivative(0).parent()
Symbolic Ring

I don't like it... though I am not sure what it should be.

@orlitzky
Copy link
Contributor

comment:19

Replying to @videlec:

I don't like it... though I am not sure what it should be.

It should be an error, because the integer zero is not a function.

But I guess that behavior is consistent with the usual abuse of notation. We convert constants to symbolic expressions so that they can be evaluated like a function. In this case, we convert ZZ(0) to SR(0), and if you think of that as being the const-zero function, its derivative is itself with respect to whatever you call the implicit argument.

@videlec
Copy link
Contributor

videlec commented Dec 29, 2021

comment:20

Replying to @orlitzky:

Replying to @videlec:

I don't like it... though I am not sure what it should be.

It should be an error, because the integer zero is not a function.

If this is your answer, then you should arguethat the derivative of any element of Integer Ring should result in an error.

But I guess that behavior is consistent with the usual abuse of notation. We convert constants to symbolic expressions so that they can be evaluated like a function. In this case, we convert ZZ(0) to SR(0), and if you think of that as being the const-zero function, its derivative is itself with respect to whatever you call the implicit argument.

Precisely, this ticket is changing that behaviour as it also make sense to take derivatives of polynomials, Laurent polynomials, power series, etc. If you find this behaviour consistent, then this ticket should not be merged at all.

@nbruin
Copy link
Contributor

nbruin commented Dec 29, 2021

comment:21

I suspect the behaviour on the integers was installed to deal with some edge cases that happened interactively or perhaps internally in SR.

As far as I can see, x.derivative(...) is meant as an application of an operator to a ring element x and the argument ... is taken as an indication what operator is meant. In particular, I don't think there is normally any effort to coerce the element x into a ring derived from the arguments ....

For instance:

sage: R.<a>=QQ[]
sage: S.<b>=QQ[]
sage: a.derivative(b)
ValueError: cannot differentiate with respect to b
sage: (a^3).derivative(2) #  2 means a repeat count here
6*a

In fact,

sage: 0.derivative(a)
AttributeError: 'sage.rings.integer.Integer' object has no attribute 'derivative'

so the functionality here is just part of the top-level derivative convenience function. Indeed, it's behaviour is: first try method application. If that fails, try if coercing into SR works. I think the current behaviour is as intended: convenience for the casual user who isn't bothered by SR. Anyone who is worried about parents should be using method calls here, since they dispatch much better among types.

These convenience functions are just not a good way of dispatching on type data. You'd have to move to a different language then, that has full signature dispatch rather than python's "first argument" dispatch through method calls.

@orlitzky
Copy link
Contributor

comment:22

Replying to @videlec:

Replying to @orlitzky:

Replying to @videlec:

I don't like it... though I am not sure what it should be.

It should be an error, because the integer zero is not a function.

If this is your answer, then you should argue that the derivative of any element of Integer Ring should result in an error.

That's right.

Precisely, this ticket is changing that behaviour as it also make sense to take derivatives of polynomials, Laurent polynomials, power series, etc. If you find this behaviour consistent, then this ticket should not be merged at all.

These top-level convenience functions that take any sage or python object are always a mess. The principled approach is to use x.derivative(), and if that method isn't there, then you can't differentiate the thing. We only run into a problem trying to teach the top-level function how to differentiate things that can't be differentiated. In that regard, asking for consistency from the result when our goals weren't consistent to begin with is too optimistic.

So I think derivative(ZZ(0)) = SR(0) is as good as any answer for the derivative of an integer. But I also think that having special cases for polynomials and series is fine too. Since polynomials overlap partially (but not completely) with things that can be coerced into SR, I don't think we'll get a nice solution that is consistent in all cases.

@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 17, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
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

6 participants