images and preimages for projective subscheme

[bhutz]

Compute the forward and inverse images for projective subschemes under projective morphisms.

The forward image can be computed with an elimination calculation and the preimage is simply composition with the map.

This includes orbit() and nth_iterate() functions for subschemes.

Component: algebraic geometry

Keywords: subscheme iteration

Author: Ben Hutz

Branch/Commit: 01073fd

Reviewer: Vincent Delecroix, Solomon Vishkautsan

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

[bhutz]

Branch: u/bhutz/ticket/19552

[sagetrac-git]

Commit: 8a0c020

[sagetrac-git]

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

8a0c020

19552: rearranged code placement, added checks

[sagetrac-git]

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

0aac0fe

19552: removed leftover code

[sagetrac-git]

Changed commit from 8a0c020 to 0aac0fe

[bhutz]

comment:4

This is ready for another set of eyes.

[bhutz]

Author: Ben Hutz

[sagetrac-git]

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

5635bec

19552: fix issue with symbolic base rings

[sagetrac-git]

Changed commit from 0aac0fe to 5635bec

[videlec]

comment:6

Hello,

1. Could you split the documentation strings into a one line description followed by a paragraph.

2. For the input, I would prefer to follow the Python conventions

• x.orbit(f, 3) -> [x, f(x), f^2(x)]

• x.orbit(f, 2, 5) -> [f^2(x), f^3(x), f^4(x)]

  In other words having a def orbit(self, f, n, m=None).

3. Is there really a need for that many functions:

• orbit (this is achieved by a simple loop)
• nth_iterate (this is more or less equivalent to .orbit(f, (n,n+1))).
• forward_image (this is more or less equivalent to .orbit(f, (0,1)))... and why not using f(self) here? If this is needed by call, then it could be a private method _forward_image.
• preimage (this should be equivalent to .orbit(f, (-1,0)))
• __call__

[videlec]

Reviewer: Vincent Delecroix

[bhutz]

comment:7

1. I will update the docs.

2. hmm..I was matching the inputs that are used for the orbit function for affine and projective points. I was not aware of the python conventions covering such a situation. If we follow them here, then the other functions should be changed to match. This is fine by me, but I'll make the other function changes in a separate ticket.

3. orbit returns a list of points and nth_iterate returns a single point. I use the functions separately as they exists for points, so I continued the separation here. Combining them would make finding an nth-iterate slightly more cumbersome: you would have to create the list orbit(f,(n,n+1)) and then get the first element out of that list. As I didn't think there was a significant drawback with making them 2 separate functions and as a user, I'd rather have two functions.

forward_image is used by call and I debated making it private or not. I think with nth_iterate existing, I can make this private.

using orbit for preimages is a little shaky in my opinion. The forward images are single points(or varieties) but the preimages are collections of points (or varieties). Allowing something like x.orbit(-2,2) would return quite a strange object. So, I see this functionality as different. In the special case of automorphisms, using orbit for both would make sense, but I don't think it does in general.

[videlec]

comment:8

Replying to @bhutz:

2. hmm..I was matching the inputs that are used for the orbit function for affine and projective points. I was not aware of the python conventions covering such a situation. If we follow them here, then the other functions should be changed to match. This is fine by me, but I'll make the other function changes in a separate ticket.

I see. Then for the sake of this ticket I guess it is better to follow the convention of the other dynamical functions. The advantage of the Python way is that it is faster to parse the argument (less type checking and avoid tuple constructions) and you have the property that

orbit(n1,n2) + orbit(n2,n3) == orbit(n1,n3)

Please let it for a later ticket (if you have time).

3. orbit returns a list of points and ...

The only draw back I see is the multiplication of methods! I do not claim that any of them is useless.

forward_image is used by call and I debated making it private or not. I think with nth_iterate existing, I can make this private.

On the other hand, nth_iterate is very hard to found with tab-completion. If I would choose one name, I would go for forward_image (with an optional argument order). For word morphism (maps of the free monoid) there is an optional argument in __call__

sage: s = WordMorphism('a->ab,b->ba')
sage: s('a')
word: ab
sage: s('a', 5)
word: abbabaabbaababbabaababbaabbabaab

But of course you can not found __call__ with tab completion. But as it is a word morphism is a function it is natural to use the my_map(x) syntax instead of x.forward_image(my_map). And actually, there is also

sage: w = Word('ab')
sage: w.apply_morphism(s)
word: abba

I guess it would be good to unify the terminology here...

using orbit for preimages is a little shaky in my opinion. The forward images are single points(or varieties) but the preimages are collections of points (or varieties).

You are right, I was thiking of bijective maps for which bi-sided orbits make sense.

Allowing something like x.orbit(-2,2) would return quite a strange object. So, I see this functionality as different. In the special case of automorphisms, using orbit for both would make sense, but I don't think it does in general.

This should be definitely disallowed in general!

[sagetrac-git]

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

45697b7

19552: adjust docstrings, rename function

[sagetrac-git]

Changed commit from 5635bec to 45697b7

[bhutz]

comment:10

Docs strings adjusted and forward_image now private. I've made a note to myself to adjust how all the orbit functions take the parameters and this will be done in different ticket. As for the naming of the functions. These match all the other dynamics functionality and for this ticket I think that is most important. If it's necessary to choose more findable names, then these should be changed for all the dynamics functionality at the same time.

[videlec]

comment:11

• In your first commit, you completely removed the file schemes/generic/morphism.py. Why is so?

• test the error that are raised. And to fit with Python standards that was discussed on sage-devel, the error message should be lower case with no ponctuation at the end like the following

sage: [][1]
Traceback (most recent call last):
...
IndexError: list index out of range

• change if (isinstance(N,(list,tuple)) == False) to if not isinstance(N, (list,tuple)). Similarly, change if normalize == True with if normalize.

• In

+        except TypeError:
+            raise TypeError("Orbit bounds must be integers")

or

+        except TypeError:
+            raise TypeError("Iterate number must be an integer")

there is no need to catch a TypeError to raise a TypeError.

• Why are you using a copy of self in orbit?

• a proejctive subscheme -> projective. Moreover the output is not a projective subscheme but a list!

• What is this line in the documentation of orbit

Perform the checks on subscheme initialization if ``check=True``

• The mention to self in the documentation should be avoided as much as possible. You can replace by this scheme.

• You can simplify a bit the following in nth_iterate

+        if n == 0:
+            return(self)
+        else:
+            Q = f(self)
+            for i in range(2,n+1):
+                Q = f(Q)
+            return(Q)

with

Q = self
for i in range(n):
    Q = f(Q)
return Q

• to make a list of zeros use [0] * n instead of [0 for _ in range(n)].

• In preimage you wrote return the subscheme why is this unique? why it does exist? The following looks fishy

sage: PS.<x,y,z> = ProjectiveSpace(ZZ, 2)
sage: H = End(PS)
sage: f = H([x^2, x^2, x^2])
sage: X = PS.subscheme([x-y])
sage: X.preimage(f)
Closed subscheme of Projective Space of dimension 2 over Integer Ring defined by:
  0
sage: f(X.preimage(f))
Closed subscheme of Projective Space of dimension 2 over Integer Ring defined by:
  y - z,
  x - z

[sagetrac-git]

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

ab4c42d

19552: fixes for orbit and nth_iterate

[bhutz]

comment:15

Corrections made. I guess I was just being overly cautious with the copy. It seems to work fine without (i.e., self remains unchanged).

I greatly increased the spectrum of failure testing as well.

Also, I was overly ambitious in my coercing in __call__ so I scaled that back slightly. I was able to produce a weird behavior with the example in algebraic_scheme.py line 2472. That example now raises an error.

As for generic/morphism.py. I couldn't find anything locally that was causing the deletion. Looking at each commit individually (locally and on trac), nowhere was it removed. Now that I've pushed these new commits up the issue seems to have gone away. I guess trac was just confused...

[videlec]

comment:16

from copy import copy is not needed anymore in algebraic_scheme.py. There exists a very nice python tool for that

$ pyflakes src/sage/schemes/generic/algebraic_scheme.py 
src/sage/schemes/generic/algebraic_scheme.py:133: 'copy' imported but unused
src/sage/schemes/generic/algebraic_scheme.py:140: 'is_MPolynomialRing' imported but unused
src/sage/schemes/generic/algebraic_scheme.py:3238: local variable 'n' is assigned to but never used

On the other hand there is still a copy in projective_point.py.

About my comment about multiplication of methods, the code in nth_iteration is an exact copy paste of a portion of orbit...

Another oneliner simplification

dict = {}
for i in range(codom.dimension_relative()+1):
    dict.update({R.gen(i): f[i]})

can be written as

dict = {R.gen(i): f[i] for i in range(codom.dimension_relative()+1)}

I have no real competence to check the mathematical validity of the code. If you want somebody else to review that part you might ask on sage-devel.

[sagetrac-git]

Changed commit from ab4c42d to 1ee5379

[sagetrac-git]

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

1ee5379

19552: remove duplicated code

[bhutz]

comment:18

ah yes, I see what you mean. I've removed the duplicated code and now call orbit from nth_iterate. I also noticed that kwds was not used in the algebraic scheme iterate, so I removed that as well.

[sagetrac-git]

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

1d65e7a

19552: add check parameter to forward and preimage

[sagetrac-git]

Changed commit from 1ee5379 to 1d65e7a

[bhutz]

comment:20

added a check parameter as the is_morphism() check can be very slow, especially for symbolic maps.

[videlec]

comment:21

It is better to make the keyword apparent in the function as in

def f(param1=True, param2=False):
    ....

instead of

def f(**kwds):
    param1 = kwds.get('param1', True)
    param2 = kwds.get('param2', False)
    ...

The reason is that with the former the tab completion works and you can see the list of keywords from the function signature.

There is still a

if (isinstance(N,(list,tuple))==False):

that should be replaced with

if not isinstance(N,(list,tuple)):

[sagetrac-git]

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

373ecb8

19552: minor correction

[sagetrac-git]

Changed commit from 1d65e7a to 373ecb8

[sagetrac-wishcow]

comment:23

It seems that the code for forward_image is implemented for any morphism Pⁿ -> P^m ^(or n-1, m-1 to be precise with the code), yet the examples are all for endomorphisms of projective space. I think it would be better to either add examples with different domain and codomain, or add an assertion that checks n=m and states otherwise that only endomorphisms are currently implemented.

[bhutz]

comment:24

Good point. I do think the mathematics is fine for n != m as long as the map is a morphism (which requires m >=n). Let me think some more about it and then I'll probably add some additional examples.

[sagetrac-wishcow]

comment:25

If I may suggest giving examples of the Veronese embedding. This is well known and people should know what their output should be. (Sorry, the Segre embedding is of cartesian product, I am guessing forward_image is not implemented for this?)

[sagetrac-git]

Changed commit from 373ecb8 to 01073fd

[sagetrac-git]

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

01073fd

19552: adding differing domain and codomain examples

[bhutz]

comment:27

Examples added. The veronese embedding is a good idea. I also did a couple dimension 0 examples for which the output is easily verified as well.

No, we can't do the segre embedding like this. However, Segre embedding is implemented for projective products.

[sagetrac-wishcow]

Changed reviewer from Vincent Delecroix to Vincent Delecroix, Solomon Vishkautsan

[vbraun]

Changed branch from u/bhutz/ticket/19552 to 01073fd