Opened 4 years ago

Closed 4 years ago

#25897 closed defect (fixed)

Incorrect Comparison of embedding index in projective_embedding

Reported by: raghukul01 Owned by:
Priority: major Milestone: sage-8.4
Component: algebraic geometry Keywords: gsoc2018
Cc: bhutz, raghukul01 Merged in:
Authors: Raghukul Raman Reviewers: Ben Hutz
Report Upstream: N/A Work issues:
Branch: 5652a9d (Commits, GitHub, GitLab) Commit: 5652a9d33cc9e050ee0273f648a26e0a91ebaa55
Dependencies: Stopgaps:

Status badges


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

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

Change History (7)

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:

786531725897: 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:

34fcf0fMerge branch 'corrected_embedding_25897' into corrected_embedding_25897_rc2
5652a9d25897: 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.