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

composition of scheme morphism defined by polynomials #15378

Closed
videlec opened this issue Nov 7, 2013 · 37 comments
Closed

composition of scheme morphism defined by polynomials #15378

videlec opened this issue Nov 7, 2013 · 37 comments

Comments

@videlec
Copy link
Contributor

videlec commented Nov 7, 2013

Currently the following fails

sage: P.<x,y>=ProjectiveSpace(QQ,1)
sage: H=Hom(P,P)
sage: f=H([x^2 -29/16*y^2, y^2])
sage: f*f
Traceback (most recent call last):
...
TypeError: unsupported operand type(s) for *: 'SchemeMorphism_polynomial_projective_space_field' and 'SchemeMorphism_polynomial_projective_space_field'

We just propose a generic implementation.

Component: algebraic geometry

Keywords: sage-days55

Author: Vincent Delecroix, Ben Hutz

Branch/Commit: c6c853f

Reviewer: Vincent Delecroix

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

@videlec videlec added this to the sage-6.1 milestone Nov 7, 2013
@videlec
Copy link
Contributor Author

videlec commented Nov 7, 2013

Author: vdelecroix

@videlec
Copy link
Contributor Author

videlec commented Nov 7, 2013

Changed keywords from none to sage-days55

@videlec
Copy link
Contributor Author

videlec commented Nov 8, 2013

New commits:

[2a78d30](https://github.com/sagemath/sagetrac-mirror/commit/2a78d30)add a *dirty* implementation of composition of scheme morphisms (see the comment in morphism.py)
[32de024](https://github.com/sagemath/sagetrac-mirror/commit/32de024)implement composition of scheme morphisms

@videlec
Copy link
Contributor Author

videlec commented Nov 8, 2013

@videlec
Copy link
Contributor Author

videlec commented Nov 8, 2013

Commit: 2a78d30

@videlec
Copy link
Contributor Author

videlec commented Nov 8, 2013

comment:3

see the discussion at
https://groups.google.com/forum/#!topic/sage-devel/qW20-IW0aks

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@bhutz
Copy link

bhutz commented Aug 2, 2016

@bhutz
Copy link

bhutz commented Aug 2, 2016

comment:8

I added the ref to the inheritance issue in #14711. It would be nice if we could have composition working independent of the inheritance structure correction. Accordingly, I put the inheritance back how it was and the composition seems to work fine with this implementation. I added one example to test failure, but that is all.

Comments?


New commits:

281d513Merge branch 7.3.beta9 into 15378
dd54bdf15378: added test and put back inheritance structure

@bhutz
Copy link

bhutz commented Aug 2, 2016

Changed commit from 2a78d30 to dd54bdf

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 2, 2016

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

e310fac15378: remove outdated todo comment

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 2, 2016

Changed commit from dd54bdf to e310fac

@videlec
Copy link
Contributor Author

videlec commented Aug 2, 2016

comment:10

You wrote

+        as soon as this bug is fixed. See trac #14711.

first of all the right syntax is :trac:`14711` and secondly #14711 refers to a closed ticket.

(EDIT: it might not be you, but it appears in the total diff.)

@videlec
Copy link
Contributor Author

videlec commented Aug 2, 2016

Changed author from vdelecroix to Vincent Delecroix, Ben Hutz

@videlec
Copy link
Contributor Author

videlec commented Aug 2, 2016

comment:11

I put in needs review in order to make a sign to the patchbots...

@bhutz
Copy link

bhutz commented Aug 2, 2016

comment:12

Yes, I can fixed the syntax mistake.

As for it being a closed ticket. In trying to investigate this issue, the comment in the code was not very helpful. The closed ticket #14711 has a much longer description of the issue and is part of what is needed. If I'm not mistaken the other part is a python bug not tracked on trac.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 2, 2016

Changed commit from e310fac to b30d9a3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 2, 2016

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

b30d9a315378: fix comment formatting

@videlec
Copy link
Contributor Author

videlec commented Aug 2, 2016

comment:14

And this can also be removed I guess

+from sage.categories.morphism import Morphism
+from sage.rings.all           import Integer
+from sage.rings.commutative_ring import is_CommutativeRing

@videlec
Copy link
Contributor Author

videlec commented Aug 2, 2016

comment:15

The correct syntax is :trac:`14711`, no braces (these are just use for formatting the trac wiki ;-)!

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 2, 2016

Changed commit from b30d9a3 to d68116e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 2, 2016

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

d68116e15378: remove extra imports

@bhutz
Copy link

bhutz commented Aug 2, 2016

comment:17

I was wondering about that trac formatting. I didn't think the docs linked to trac like that, but I figured if you said so... ;)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 2, 2016

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

133632f15378: fix doc formatting

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 2, 2016

Changed commit from d68116e to 133632f

@videlec
Copy link
Contributor Author

videlec commented Aug 3, 2016

comment:19

Note that even without your branch it (sort of) works!

sage: P.<x,y> = ProjectiveSpace(QQ,1)
sage: H = Hom(P,P)
sage: f = H([x^2 -29/16*y^2, y^2])
sage: g = H([y,x+y])
sage: h = f*g
sage: h
Composite map:
  From: Projective Space of dimension 1 over Rational Field
  To:   Projective Space of dimension 1 over Rational Field
  Defn:   Generic endomorphism of Projective Space of dimension 1 over Rational Field
        then
          Generic endomorphism of Projective Space of dimension 1 over Rational Field
sage: p = P((1,3))
sage: p
(1/3 : 1)
sage: h(p) == f(g(p))
True

But I guess it is better to have the result of f*g with explicit polynomial coordinates.

sage: f[0]
x^2 - 29/16*y^2
sage: h[0]
Generic endomorphism of Projective Space of dimension 1 over Rational Field

@videlec
Copy link
Contributor Author

videlec commented Aug 3, 2016

comment:20

I guess that the lines

except AttributeError:
    raise NotImplementedError

would better be replaced with

except AttributeError:
    return super(SchemeMorphism_polynomial, self)._composition_(self, other, homset)

as there might be some morphism other than polynomial that you may want to compose with polynomial ones. I was not able to build example though.

@bhutz
Copy link

bhutz commented Aug 3, 2016

comment:21

This composition is specifically for SchemeMorphism_polynomial, so I'm fine with the NotImplementedError if there are not any polynomials. If it is a different kind of morphism, it shouldn't be calling this one.

Yes, getting the defining polynomials of the composition really the point. We needed this to speed up the computations for things like #21118.

@videlec
Copy link
Contributor Author

videlec commented Aug 3, 2016

comment:22

More precisely: f can be polynomial and g something else and f * g should work.

@bhutz
Copy link

bhutz commented Aug 3, 2016

comment:23

This gets the NotImplementedError

A2.<x,y> = AffineSpace(QQ,2)
H=End(A2)
f=H([x^2+y^2,y^2/x])
p=A2([1,2])
f*p

@videlec
Copy link
Contributor Author

videlec commented Aug 3, 2016

comment:24

I also built another example with Galois conjugation

x = polygen(QQ)
K.<a> = NumberField(x^2 - 2)
p1 ,p2 = K.Hom(K)

R.<x,y> = K[]
q1 = R.Hom(R)(p1)
q2 = R.Hom(R)(p2)

A = AffineSpace(R)
f1 = A.Hom(A)(q1)
f2 = A.Hom(A)(q2)
g = A.Hom(A)([x^2-y, y+1])
f1*g  # fine
g*f1  # NotImplementedError

@bhutz
Copy link

bhutz commented Aug 3, 2016

comment:25

If I make your code

  return super(SchemeMorphism_polynomial, self)._composition_(other, homset)

Then I get

sage: A2.<x,y> = AffineSpace(QQ,2)
sage: H=End(A2)
sage: f=H([x^2+y^2,y^2/x])
sage: p= A2([1,2])
sage: f*p         	
Composite map:
  From: Spectrum of Rational Field
  To:   Affine Space of dimension 2 over Rational Field
  Defn:   Generic morphism:
          From: Spectrum of Rational Field
          To:   Affine Space of dimension 2 over Rational Field
        then
          Generic endomorphism of Affine Space of dimension 2 over
Rational Field

However, I can't seem to actually apply the map

sage: p(Spec(QQ).an_element())
TypeError: Point on Spectrum of Rational Field defined by the Principal ideal (0) of Rational Field fails to convert into the map's domain Spectrum of Rational Field, but a `pushforward` method is not properly implemented

With your new example, I now get:

Composite map:
  From: Affine Space of dimension 2 over Number Field in a with defining
polynomial x^2 - 2
  To:   Affine Space of dimension 2 over Number Field in a with defining
polynomial x^2 - 2
  Defn:   Generic endomorphism of Affine Space of dimension 2 over
Number Field in a with defining polynomial x^2 - 2
        then
          Generic endomorphism of Affine Space of dimension 2 over
Number Field in a with defining polynomial x^2 - 2

I haven't been able to evaluate yours at a pt yet...

@videlec
Copy link
Contributor Author

videlec commented Aug 3, 2016

comment:26

For Galois conjugation the point is that the ring homomorphism f1 lacks a inverse_image for ideals.

There is something weird with the composition telling that it is the composition of two Generic endomorphisms.

And an other error

sage: Ai = A.Hom(A).identity()
sage: f = A.Hom(A)([x,y])
sage: Ai * f  # TypeError
sage: f * Ai  # TypeError

I opened #21160 for the above since it is distinct from what we are trying to do here...

@bhutz
Copy link

bhutz commented Aug 3, 2016

comment:27

I do think allowing this general morphism composition is better, so I'll push that change shortly

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2016

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

c6c853f15378: allow general compositions poly*notpoly

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2016

Changed commit from 133632f to c6c853f

@videlec
Copy link
Contributor Author

videlec commented Aug 3, 2016

Reviewer: Vincent Delecroix

@vbraun
Copy link
Member

vbraun commented Aug 7, 2016

Changed branch from u/bhutz/15378-scheme_morphism_composition to c6c853f

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