Opened 5 years ago

Closed 2 years ago

#15378 closed enhancement (fixed)

composition of scheme morphism defined by polynomials

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-7.4
Component: algebraic geometry Keywords: sage-days55
Cc: Merged in:
Authors: Vincent Delecroix, Ben Hutz Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: c6c853f (Commits) Commit: c6c853fa06cf3ff5a08b64cc58e71d39e72f271e
Dependencies: Stopgaps:

Description

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.

Change History (30)

comment:1 Changed 5 years ago by vdelecroix

  • Authors set to vdelecroix
  • Component changed from PLEASE CHANGE to algebraic geometry
  • Keywords sage-days55 added
  • Type changed from PLEASE CHANGE to enhancement

comment:2 Changed 5 years ago by vdelecroix

  • Branch set to u/vdelecroix/15378-scheme_morphism_composition
  • Commit set to 2a78d304076962893c004184a5b6e3d070ed158d

New commits:

2a78d30add a *dirty* implementation of composition of scheme morphisms (see the comment in morphism.py)
32de024implement composition of scheme morphisms

comment:4 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:5 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:6 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:7 Changed 2 years ago by bhutz

  • Branch changed from u/vdelecroix/15378-scheme_morphism_composition to u/bhutz/15378-scheme_morphism_composition

comment:8 Changed 2 years ago by bhutz

  • Commit changed from 2a78d304076962893c004184a5b6e3d070ed158d to dd54bdfb9c874ace2ccc487a8abcb5784e505ed8

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

comment:9 Changed 2 years ago by git

  • Commit changed from dd54bdfb9c874ace2ccc487a8abcb5784e505ed8 to e310fac2c3f76e470925250a029b2c774ae7a37d

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

e310fac15378: remove outdated todo comment

comment:10 Changed 2 years ago by vdelecroix

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.)

Last edited 2 years ago by vdelecroix (previous) (diff)

comment:11 Changed 2 years ago by vdelecroix

  • Authors changed from vdelecroix to Vincent Delecroix, Ben Hutz
  • Status changed from new to needs_review

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

comment:12 Changed 2 years ago by bhutz

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.

comment:13 Changed 2 years ago by git

  • Commit changed from e310fac2c3f76e470925250a029b2c774ae7a37d to b30d9a33176283eb308487aa17e72c3abf9cecf2

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

b30d9a315378: fix comment formatting

comment:14 Changed 2 years ago by vdelecroix

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

comment:15 Changed 2 years ago by vdelecroix

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

comment:16 Changed 2 years ago by git

  • Commit changed from b30d9a33176283eb308487aa17e72c3abf9cecf2 to d68116e1fcce8de963f4d83acaac4f0daa4ffe60

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

d68116e15378: remove extra imports

comment:17 Changed 2 years ago by bhutz

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... ;)

comment:18 Changed 2 years ago by git

  • Commit changed from d68116e1fcce8de963f4d83acaac4f0daa4ffe60 to 133632f7da656d28d81067609637e58785d0c6a5

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

133632f15378: fix doc formatting

comment:19 Changed 2 years ago by vdelecroix

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

comment:20 Changed 2 years ago by vdelecroix

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.

comment:21 Changed 2 years ago by bhutz

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.

comment:22 Changed 2 years ago by vdelecroix

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

comment:23 Changed 2 years ago by bhutz

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

comment:24 Changed 2 years ago by vdelecroix

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

comment:25 Changed 2 years ago by bhutz

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...

comment:26 Changed 2 years ago by vdelecroix

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...

comment:27 Changed 2 years ago by bhutz

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

comment:28 Changed 2 years ago by git

  • Commit changed from 133632f7da656d28d81067609637e58785d0c6a5 to c6c853fa06cf3ff5a08b64cc58e71d39e72f271e

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

c6c853f15378: allow general compositions poly*notpoly

comment:29 Changed 2 years ago by vdelecroix

  • Milestone changed from sage-6.4 to sage-7.4
  • Reviewers set to Vincent Delecroix
  • Status changed from needs_review to positive_review

comment:30 Changed 2 years ago by vbraun

  • Branch changed from u/bhutz/15378-scheme_morphism_composition to c6c853fa06cf3ff5a08b64cc58e71d39e72f271e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.