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

Univariate Laurent polynomial do not properly handle __call__ #17554

Closed
tscrim opened this issue Dec 27, 2014 · 10 comments
Closed

Univariate Laurent polynomial do not properly handle __call__ #17554

tscrim opened this issue Dec 27, 2014 · 10 comments

Comments

@tscrim
Copy link
Collaborator

tscrim commented Dec 27, 2014

Univariate Laurent polynomials behave very differently with __call__ compared to other polynomials. In particular, the following does not (correctly) work:

sage: R.<t> = LaurentPolynomialRing(ZZ)
sage: f = t^(-2) + t^2
sage: f(t=-1)  # Boom
sage: f(x=-1)  # Boom
sage: f()  # Boom
sage: f(1,2)  # Should be an error
2

The original symptom (which has been fixed by other means, see comment:3) came from

sage: R.<q> = QQ[]
sage: p = q^4 + q^2 - 2*q + 3
sage: L.<x,y> = LaurentPolynomialRing(QQ)
sage: p(q=x)
x^4 + x^2 - 2*x + 3

but if we change things to a univariate Laurent polynomial ring, we get:

sage: L.<x> = LaurentPolynomialRing(QQ)
sage: p(q=x)
...
IndexError: tuple index out of range

See comment:2.

Component: commutative algebra

Keywords: Laurent polynomial, substitution

Author: Travis Scrimshaw

Branch/Commit: 7f41391

Reviewer: Frédéric Chapoton

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

@tscrim tscrim added this to the sage-6.5 milestone Dec 27, 2014
@tscrim tscrim self-assigned this Dec 27, 2014
@tscrim tscrim changed the title Unable to substitute univariant Laurent polynomial into usual polynomial Unable to substitute univariate Laurent polynomial into usual polynomial Dec 27, 2014
@nbruin
Copy link
Contributor

nbruin commented Dec 27, 2014

comment:2

The following seems to be the issue

sage: p() #evaluating with no arguments should probably be a no-op
q^4 + q^2 - 2*q + 3
sage: x() #but it raises an error for laurent polynomials
IndexError: tuple index out of range

The problem gets triggered by sage.rings.polynomial.polynomial_element.Polynomial.__call__, which for keyword arguments does:

          P = self.parent()
            name = P.variable_name()
            if name in kwds:
                if len(x) > 0:
                    raise ValueError("must not specify both a keyword and positional argument")
                a = self(kwds[name])
                del kwds[name]
                try:
                    return a(**kwds)
                except TypeError:
                    return a

The command return a(**kwds) fails, because it's effectively a(*(),**{}). Furthermore, it fails with an IndexError which doesn't get caught.

The better solution is probably to amend laurentpolynomial's __call__ to do the right thing. Presently, it doesn't support keyword arguments at all and it expects non-empty arguments. Its implementation is

    def __call__(self, *x):
        if isinstance(x[0], tuple):
            x = x[0]
        return self.__u(x) * (x[0]**self.__n)

which expects there is at least one argument and doesn't handle keyword arguments.

@fchapoton
Copy link
Contributor

comment:3

This seems to work now (8.0.b11)

@tscrim
Copy link
Collaborator Author

tscrim commented Jun 24, 2017

comment:4

However, there are still serious issues with __call__ that I am recycling this ticket for (sorry it completely fell off my radar). The attached branch makes the behavior standard with the rest of polynomials Sage (with some mild cleanup of the multivariate __call__).


New commits:

7f41391Make univariate Laurent polynomials behave like other polynomials.

@tscrim

This comment has been minimized.

@tscrim
Copy link
Collaborator Author

tscrim commented Jun 24, 2017

Author: Travis Scrimshaw

@tscrim
Copy link
Collaborator Author

tscrim commented Jun 24, 2017

Commit: 7f41391

@tscrim
Copy link
Collaborator Author

tscrim commented Jun 24, 2017

@tscrim tscrim changed the title Unable to substitute univariate Laurent polynomial into usual polynomial Univariate Laurent polynomial do not properly handle __call__ Jun 24, 2017
@tscrim tscrim modified the milestones: sage-6.5, sage-8.0 Jun 24, 2017
@fchapoton
Copy link
Contributor

comment:5

ok, let it be. Thanks

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@vbraun
Copy link
Member

vbraun commented Jun 25, 2017

Changed branch from public/rings/laurent_polynomial_call-17554 to 7f41391

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

4 participants