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

conversion in multivariate polynomial ring fails #23629

Open
dkrenn opened this issue Aug 17, 2017 · 4 comments
Open

conversion in multivariate polynomial ring fails #23629

dkrenn opened this issue Aug 17, 2017 · 4 comments

Comments

@dkrenn
Copy link
Contributor

dkrenn commented Aug 17, 2017

sage: K = QQ
sage: QQabc = PolynomialRing(K, ['a', 'b', 'c'])
sage: QQxy = PolynomialRing(K, ['x', 'y'])
sage: Pxy = PolynomialRing(QQabc, ['x', 'y'])
sage: Pabc = PolynomialRing(QQxy, ['a', 'b', 'c'])
sage: Pabc(QQabc.0 * Pxy.1)

fails with a TypeError.

Component: algebra

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

@dkrenn dkrenn added this to the sage-8.1 milestone Aug 17, 2017
@dkrenn
Copy link
Contributor Author

dkrenn commented Aug 17, 2017

comment:1

Maybe #22333 would fix this issue.

@tscrim

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Aug 17, 2017

comment:2

To begin, we can simplify this a little bit by taking K = QQ. I updated the description accordingly.

I don't think that #22333 would fix the issue as I think the conversion is getting confused. Actually, this code block in _mpoly_dict_recursive() seems to be where problems start:

        elif not my_vars[-1] in vars:
            x = base_ring(self) if base_ring is not None else self
            const_ix = ETuple((0,)*len(vars))
            return { const_ix: x }

You get into a chicken-and-egg problem of needing to extract out all of your variables from the input x, but first you need to know there are none of your variables in x as a polynomial in its current form. This is what leads to the current problem, it thinks a*y should be treated as a constant polynomial with respect to the variables abc. The recurse is made and it would then extract the y variable, but it doesn't know what to do with the a because the base ring is K. This is what raises the error.

I feel like the problem is that the polynomial code that calls this only passes variable_names() rather than variable_names_recursive(). However, even with doing that, it doesn't extract the correct variables first due to the ordering of variable_names_recursive() with your variables being last. I think the code that tries to construct the mapping is making way too many assumptions about the input, especially for how general I feel this code should work.

Thus, I think two things need to be fixed. The first is that we should be using variable_names_recursive() when calling _mpoly_dict_recursive in __call__. The second is that the handling of variables when my_vars is a subset of vars needs to be done without assuming my_vars are the last variables of vars.


Additionally, this "works" however:

sage: K = QQ
sage: QQabc = PolynomialRing(K, ['a', 'b', 'c'])
sage: QQxyz = PolynomialRing(K, ['x', 'y', 'z'])
sage: Pxyz = PolynomialRing(QQabc, ['x', 'y', 'z'])
sage: Pabc = PolynomialRing(QQxyz, ['a', 'b', 'c'])
sage: Pabc(QQabc.0 * Pxyz.1)
x*b

because it goes through a slightly different code path. However, the result is wrong:

sage: QQabc.0 * Pxyz.1
a*y

This result is because it first matches the number of variables and does:

sage: QQabc(QQxyz.1)
b

So this may even be considered "correct" behavior, but it is deserving of a sage-devel ask and would be a separate ticket (if considered a bug).

@dkrenn
Copy link
Contributor Author

dkrenn commented Aug 17, 2017

comment:3

See also #23631.

@mkoeppe mkoeppe removed this from the sage-8.1 milestone Dec 29, 2022
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

3 participants