Opened 4 years ago
Closed 4 years ago
#16834 closed defect (fixed)
Change ring fails for affine morphisms.
Reported by:  gjorgenson  Owned by:  gjorgenson 

Priority:  minor  Milestone:  sage6.4 
Component:  algebraic geometry  Keywords:  
Cc:  bhutz  Merged in:  
Authors:  Grayson Jorgenson  Reviewers:  Ben Hutz 
Report Upstream:  N/A  Work issues:  
Branch:  4b84926 (Commits)  Commit:  4b8492632a6064f3396c7a49815b1bffa161ded0 
Dependencies:  Stopgaps: 
Description (last modified by )
When calling the change ring function on affine morphisms an index out of bounds error occurs:
sage: A.<x,y,z> = AffineSpace(RR,3) sage: h = Hom(A,A) sage: f = h([x^2+1.5,y^3,z^52.0]) sage: f.change_ring(CC)
Change History (17)
comment:1 Changed 4 years ago by
 Branch set to u/gjorgenson/ticket/16834
 Created changed from 08/15/14 18:01:08 to 08/15/14 18:01:08
 Modified changed from 08/15/14 18:01:08 to 08/15/14 18:01:08
comment:2 Changed 4 years ago by
 Commit set to d3075c649bf1d23c07bed646775f7ed84fe4d7eb
 Description modified (diff)
 Status changed from new to needs_review
comment:3 Changed 4 years ago by
 Status changed from needs_review to needs_work
I'd like to see the example a little more complex. Say at least dimension 2 with nonconstant polynomials.
For the code itself you can get rid of the helper list F with the following
G=[F.change_ring(R) for F in self._polys]
Another thought. What happens when the domain and codomain are different? This code is assuming that the domain and codomain are the same.
comment:4 Changed 4 years ago by
 Reviewers set to Ben Hutz
comment:5 Changed 4 years ago by
 Commit changed from d3075c649bf1d23c07bed646775f7ed84fe4d7eb to 0a29b1b9788b97fa7bbd20d9443afe7118be673a
Branch pushed to git repo; I updated commit sha1. New commits:
0a29b1b  Minor changes in morphism.py to import list and change_ring for scheme polynomial morphisms

comment:6 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:7 Changed 4 years ago by
 Description modified (diff)
comment:8 Changed 4 years ago by
 Status changed from needs_review to needs_work
I think the endomorphism check needs some work, since we really want is
instead of ==
. Here is a suggestion to simplify things:
T=self.domain().change_ring(R) if self.is_endomorphism(): H=End(T) else: H=Hom(T,self.codomain().change_ring(R))
comment:9 Changed 4 years ago by
 Commit changed from 0a29b1b9788b97fa7bbd20d9443afe7118be673a to ecc938e4adf473ee0cae75e67d279bbe18a120f7
Branch pushed to git repo; I updated commit sha1. New commits:
ecc938e  Minor change in change ring function for polynomial scheme morphisms

comment:10 Changed 4 years ago by
 Status changed from needs_work to needs_review
Is the use of is_endomorphism()
more sound than using ==
for the comparison of the domain and codomain?
comment:11 Changed 4 years ago by
I think it is faster, since you don't have to access and change_ring the codomain unless you really need it and it makes the code easily human readable.
The functionality looks fine at this point, but while you're fixing this function, there are a couple other things to fix
1) there are many instances of trailing white space that need to be removed
2) fix the formatting for the earlier examples for this function
3) the output doesn't seem like an accurate description. Perhaps something more like in the description: a :class:SchemeMorphism_polynomial
which is self
coerced to R
comment:12 Changed 4 years ago by
 Commit changed from ecc938e4adf473ee0cae75e67d279bbe18a120f7 to ef35bccf56617d62ec88d9c94f738984a7788893
Branch pushed to git repo; I updated commit sha1. New commits:
ef35bcc  Minor changes to change ring function for polynomial scheme morphisms

comment:13 Changed 4 years ago by
 Branch changed from u/gjorgenson/ticket/16834 to u/bhutz/ticket/16834
 Modified changed from 08/20/14 16:53:51 to 08/20/14 16:53:51
comment:14 Changed 4 years ago by
 Commit changed from ef35bccf56617d62ec88d9c94f738984a7788893 to 4b8492632a6064f3396c7a49815b1bffa161ded0
 Status changed from needs_review to positive_review
Just removed some extra/trailing white space. This looks good.
New commits:
4b84926  removed extra whitespace

comment:16 Changed 4 years ago by
 Status changed from needs_work to positive_review
comment:17 Changed 4 years ago by
 Branch changed from u/bhutz/ticket/16834 to 4b8492632a6064f3396c7a49815b1bffa161ded0
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Minor change in change_ring function for scheme morphisms.