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:  sage7.4 
Component:  algebraic geometry  Keywords:  sagedays55 
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
 Component changed from PLEASE CHANGE to algebraic geometry
 Keywords sagedays55 added
 Type changed from PLEASE CHANGE to enhancement
comment:2 Changed 5 years ago by
 Branch set to u/vdelecroix/15378scheme_morphism_composition
 Commit set to 2a78d304076962893c004184a5b6e3d070ed158d
comment:3 Changed 5 years ago by
see the discussion at https://groups.google.com/forum/#!topic/sagedevel/qW20IW0aks
comment:4 Changed 5 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:5 Changed 5 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:6 Changed 4 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:7 Changed 2 years ago by
 Branch changed from u/vdelecroix/15378scheme_morphism_composition to u/bhutz/15378scheme_morphism_composition
comment:8 Changed 2 years ago by
 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:
281d513  Merge branch 7.3.beta9 into 15378

dd54bdf  15378: added test and put back inheritance structure

comment:9 Changed 2 years ago by
 Commit changed from dd54bdfb9c874ace2ccc487a8abcb5784e505ed8 to e310fac2c3f76e470925250a029b2c774ae7a37d
Branch pushed to git repo; I updated commit sha1. New commits:
e310fac  15378: remove outdated todo comment

comment:10 Changed 2 years ago by
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.)
comment:11 Changed 2 years ago by
 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
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
 Commit changed from e310fac2c3f76e470925250a029b2c774ae7a37d to b30d9a33176283eb308487aa17e72c3abf9cecf2
Branch pushed to git repo; I updated commit sha1. New commits:
b30d9a3  15378: fix comment formatting

comment:14 Changed 2 years ago by
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
The correct syntax is :trac:`14711`
, no braces (these are just use for formatting the trac wiki ;)!
comment:16 Changed 2 years ago by
 Commit changed from b30d9a33176283eb308487aa17e72c3abf9cecf2 to d68116e1fcce8de963f4d83acaac4f0daa4ffe60
Branch pushed to git repo; I updated commit sha1. New commits:
d68116e  15378: remove extra imports

comment:17 Changed 2 years ago by
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
 Commit changed from d68116e1fcce8de963f4d83acaac4f0daa4ffe60 to 133632f7da656d28d81067609637e58785d0c6a5
Branch pushed to git repo; I updated commit sha1. New commits:
133632f  15378: fix doc formatting

comment:19 Changed 2 years ago by
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
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
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
More precisely: f
can be polynomial and g
something else and f * g
should work.
comment:23 Changed 2 years ago by
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
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^2y, y+1]) f1*g # fine g*f1 # NotImplementedError
comment:25 Changed 2 years ago by
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
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
I do think allowing this general morphism composition is better, so I'll push that change shortly
comment:28 Changed 2 years ago by
 Commit changed from 133632f7da656d28d81067609637e58785d0c6a5 to c6c853fa06cf3ff5a08b64cc58e71d39e72f271e
Branch pushed to git repo; I updated commit sha1. New commits:
c6c853f  15378: allow general compositions poly*notpoly

comment:29 Changed 2 years ago by
 Milestone changed from sage6.4 to sage7.4
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to positive_review
comment:30 Changed 2 years ago by
 Branch changed from u/bhutz/15378scheme_morphism_composition to c6c853fa06cf3ff5a08b64cc58e71d39e72f271e
 Resolution set to fixed
 Status changed from positive_review to closed
New commits: