class for flattening polynomial rings over polynomial rings

[bhutz]

Given a polynomial ring QQ['a',b']['x','y'] construct a morphism and its inverse to the ring QQ['a','b','x','y']

CC: @videlec

Component: algebra

Author: Ben Hutz, Vincent Delecroix

Branch/Commit: 19cb171

Reviewer: Vincent Delecroix

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

[bhutz]

Branch: u/bhutz/flatten

[bhutz]

comment:2

Here is a first attempt to flesh this out. Please comment.

---

New commits:

cf97fed

21106: create polynomial ring flattening class

[bhutz]

Commit: cf97fed

[videlec]

comment:4

For example ``QQ['a','b'],['x','y']`` flattens to ``QQ['a','b','c','d']``.

should I read ``QQ['a', 'b', 'x', 'y']``?

[videlec]

comment:5

The error ValueError("clash in variable names") should be tested.

[videlec]

comment:6

I am not sure we want that FlatteningMorphism in the global namespace.

[bhutz]

comment:7

Those are all simple enough. I also removed some terminating white space

[videlec]

comment:8

Here is a recursive version of _call_ that seems to be a little bit faster

    def _call2_(self, p):
        r"""
        TESTS::

            sage: R = QQ['x']['y']['s','t']
            sage: p = R('s*x + y*t + x^2*s + 1 + t')
            sage: f = FlatteningMorphism(R)
            sage: f._call2_(p)
            x^2*s + x*s + y*t + t + 1
        """
        p = {(): p}

        for ring in self._intermediate_rings:
            new_p = {}
            if is_PolynomialRing(ring):
                for mon,pp in p.iteritems():
                    assert pp.parent() == ring
                    for i,j in pp.dict().iteritems():
                        new_p[(i,)+(mon)] = j
            elif is_MPolynomialRing(ring):
                for mon,pp in p.iteritems():
                    assert pp.parent() == ring
                    for mmon,q in pp.dict().iteritems():
                        new_p[tuple(mmon)+mon] = q
            else:
                raise RuntimeError
            p = new_p

    return self.codomain()(p)

[sagetrac-git]

Changed commit from cf97fed to 957589f

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

957589f

21106: minor fixes

[videlec]

comment:10

To use _call2_, the constructor needs to be modified with

        intermediate_rings = []
        ... # AS BEFORE
        while is_PolynomialRing(ring) or is_MPolynomialRing(ring):
            intermediate_rings.append(ring)
            ... # AS BEFORE
        ... # AS BEFORE
        self._intermediate_rings = intermediate_rings

[videlec]

comment:11

Do you think we want to support both version of _call_?

[bhutz]

comment:12

None of the examples that I've tried fail for call2? The more constructive call2 sits better with me as it is not relying on iterpreting a string as a polynomial.

[videlec]

comment:13

Note that a similar _call_ can be implemented for the section morphism.

[sagetrac-git]

Changed commit from 957589f to 8c22f75

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

8c22f75

21106: new version of _call_

[bhutz]

comment:15

changes made and a version of call2 for unflattening. Not quite as elegant as yours, but constructive.

[videlec]

comment:16

Is it faster than string?

[bhutz]

comment:17

much faster for things like CC and str does not work for QQbar. It is slightly slower for QQ.

[videlec]

comment:18

Replying to @bhutz:

much faster for things like CC and str does not work for QQbar. It is slightly slower for QQ.

Would be good to add doctest for QQbar. By the way, the string method would also fail for something like the following (that would be good to doctest as well)

sage: K.<a> = NumberField(x^3 - 2)
sage: R = K['x','y']['a','b']

[videlec]

comment:19

Strings are also bad for floating point

sage: a = RR(1 / 2**30)
sage: RR(str(a)) == a
False

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

570d9ee

21106: added doc tests

[sagetrac-git]

Changed commit from 8c22f75 to 570d9ee

[bhutz]

comment:21

yes, those are good and caught an error in the codomain as well that I've corrected.

[videlec]

New commits:

f05f7db

21106:cleaning

[videlec]

comment:26

problem with the file mode... it is -rwxr-xr-x instead of -rw-r--r--.

(incidentally I have another commit to propose)

[sagetrac-git]

Changed commit from f05f7db to 19cb171

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

6e50942	21106: change file mode
19cb171	21106: Python3 compatibility + more cleaning

[bhutz]

comment:29

those changes are fine with me. I pulled it down to test just to be safe, and all still runs fine.

[videlec]

comment:30

Thanks!

I also think that in the future the FlatteningMorphism should support QQ['a','b']['a','b']['a']['b']. There is almost nothing to modify to make the target to be QQ['x0','x1','x2','x3','x4','x5'] (I guess only the constructor).

[bhutz]

comment:31

You probably saw, I removed the original codomain argument. I was running into some difficulties there with _call_, but with the new version of _call_ it would probably actually be easy now.

[videlec]

comment:32

Yep. It might also be that for some operation it is good to try in QQ['a']['b']['c']['d']['e'] instead. In which case we might want to use UnflattenMorphism without using FlattenMorphism. See my recent post

https://groups.google.com/forum/#!topic/sage-devel/CDLCb6OX4ns

[bhutz]

comment:33

unflatteningmorphism cannot be used by itself since _intermediate rings is defined through section. I'll fix that shortly.

[videlec]

comment:34

You are free to open a new ticket for that issue. Do not change a ticket in positive_review to needs_work. Moreover when it is in that state since a long time and that there are other tickets based on it.

[bhutz]

comment:35

ok, so I should mark this back to positive, and then open a new ticket to fix the issue?

[videlec]

comment:36

better, yes.

[bhutz]

comment:37

the issue is now #21113

[vbraun]

Changed branch from public/21106 to 19cb171