Opened 4 years ago

Closed 4 years ago

# Incorrect Comparison of embedding index in projective_embedding

Reported by: Owned by: raghukul01 major sage-8.4 algebraic geometry gsoc2018 bhutz, raghukul01 Raghukul Raman Ben Hutz N/A 5652a9d 5652a9d33cc9e050ee0273f648a26e0a91ebaa55

### Description

Consider this example:

```sage: A.<x,y> = AffineSpace(ZZ, 2)
sage: A.projective_embedding(4)
1 A = AffineSpace(ZZ, Integer(2), names=('x', 'y',)); (x, y,) = A._first_ngens(2)
----> 2 A.projective_embedding(Integer(3))

/home/raghukul/sage/sage/local/lib/python2.7/site-packages/sage/schemes/affine/affine_space.pyc in projective_embedding(self, i, PP)
724         self.__projective_embedding[i] = phi
725         #make affine patch and projective embedding match
--> 726         PP.affine_patch(i,self)
727         return phi
728

/home/raghukul/sage/sage/local/lib/python2.7/site-packages/sage/schemes/projective/projective_space.pyc in affine_patch(self, i, AA)
978         n = self.dimension_relative()
979         if i < 0 or i > n:
--> 980             raise ValueError("argument i (= %s) must be between 0 and %s"%(i, n))
981         try:
982             A = self.__affine_patches[i]

ValueError: argument i (= 3) must be between 0 and 2
```

This however returns a `ValueError`, but only when the affine patch is generated for corresponding `ProjectiveSpace`. It simply skips the check for index in projective_embedding().

Following line in projective_embedding needs to be corrected:

```        if n < 0 or n >self.dimension_relative():
raise ValueError("argument i (=%s) must be betwzzeen 0 and %s, inclusive"%(i,n))
```

### comment:1 Changed 4 years ago by raghukul01

• Branch set to u/raghukul01/corrected_embedding_25897

### comment:2 Changed 4 years ago by raghukul01

• Authors set to Raghukul Raman
• Commit set to 786531723a5589f32758325e8f15332f6d1969ca
• Status changed from new to needs_review

New commits:

 ​7865317 `25897: Corrected projective_embedding function`

### comment:3 Changed 4 years ago by bhutz

• Reviewers set to Ben Hutz
• Status changed from needs_review to needs_work

You should add a doctest item under TESTS for that error message.

### comment:4 Changed 4 years ago by git

• Commit changed from 786531723a5589f32758325e8f15332f6d1969ca to 5652a9d33cc9e050ee0273f648a26e0a91ebaa55

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

 ​34fcf0f `Merge branch 'corrected_embedding_25897' into corrected_embedding_25897_rc2` ​5652a9d `25897: Added doctest to illustrate fix`

### comment:5 Changed 4 years ago by raghukul01

• Status changed from needs_work to needs_review

Added `TESTS` block

### comment:6 Changed 4 years ago by bhutz

• Status changed from needs_review to positive_review

### comment:7 Changed 4 years ago by vbraun

• Branch changed from u/raghukul01/corrected_embedding_25897 to 5652a9d33cc9e050ee0273f648a26e0a91ebaa55
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.