Opened 8 years ago

Closed 7 years ago

#16158 closed enhancement (fixed)

Make Spec into a functor

Reported by: pbruin Owned by:
Priority: major Milestone: sage-6.3
Component: algebraic geometry Keywords: Spec functor affine scheme
Cc: nthiery, vbraun Merged in:
Authors: Peter Bruin Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 185b49c (Commits, GitHub, GitLab) Commit: 185b49cd73907d2214401f96058e61b0eb334d8e
Dependencies: #15990, #16156, #16680 Stopgaps:

Status badges

Description (last modified by pbruin)

Sage's Spec command currently produces a Spec object that derives from, but is not the same as, an AffineScheme. The goal of this ticket is

  • merge the existing Spec with AffineScheme by moving all existing methods of Spec to AffineScheme;
  • upgrade Spec to a functor from CommutativeRings to Schemes (or Schemes(A) if a base ring A is specified), returning objects of type AffineScheme.

Example of the new functionality:

sage: A.<x,y> = QQ[]
sage: Spec(A)
Spectrum of Multivariate Polynomial Ring in x, y over Rational Field
sage: type(Spec(A))
<class 'sage.schemes.generic.scheme.AffineScheme_with_category'>
sage: B.<t> = QQ[]
sage: f = A.hom((t^2, t^3))
sage: Spec(f)
Affine Scheme morphism:
  From: Spectrum of Univariate Polynomial Ring in t over Rational Field
  To:   Spectrum of Multivariate Polynomial Ring in x, y over Rational Field
  Defn: Ring morphism:
          From: Multivariate Polynomial Ring in x, y over Rational Field
          To:   Univariate Polynomial Ring in t over Rational Field
          Defn: x |--> t^2
                y |--> t^3

Two small user-visible changes had to be made to accommodate the new situation:

  • If S = Spec(A) is an affine scheme, then the syntax S(a_1, ..., a_n) to construct the topological point of S defined by the prime ideal P = (a1, ..., an) of A is no longer supported. The syntax S(A.ideal(a_1, ..., a_n)) now has to be used instead. This is because it conflicts with the much more useful application of this syntax to construct the point with coordinates (a1, ..., an) if S is (a subscheme of) an affine space An.
  • Given S = Spec(A) and another scheme X, the result of X(A) is the same as before (a point homset), but X(S), which used to be identical to this, now returns the standard scheme homset. To get the point homset, one now has to type X(A) or X(S.coordinate_ring()). This seems the "principle of least surprise" convention to me, and it is consistent with the fact that X.point_homset() only accepts rings, not affine schemes.

More improvements to affine schemes are made in #7946.

Change History (21)

comment:1 Changed 8 years ago by pbruin

  • Branch set to u/pbruin/16158-Spec_functor
  • Commit set to 9d4ddc6bbd7a98e8312d93e6c85a44c4bc6cd560
  • Status changed from new to needs_review

For easier reviewing (until #15990 is merged), it is probably helpful to look at the individual commits or at the output of git diff bf9f3f4a 68f25537. [Edit: not necessary anymore.]

Last edited 8 years ago by pbruin (previous) (diff)

comment:2 Changed 8 years ago by git

  • Commit changed from 9d4ddc6bbd7a98e8312d93e6c85a44c4bc6cd560 to cb82f0114374ea03d8c970d8bbd0972f0f5637f8

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

d87d5a8Merge branch 'develop' into ticket/16158-Spec_functor
cb82f01fix raise syntax and a line of documentation

comment:3 Changed 8 years ago by git

  • Commit changed from cb82f0114374ea03d8c970d8bbd0972f0f5637f8 to dcf608f261a4b2efa8e1f5195713e6a676366c07

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

dcf608fMerge branch 'develop' into ticket/16158-Spec_functor

comment:4 Changed 7 years ago by pbruin

  • Description modified (diff)

comment:5 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:6 Changed 7 years ago by git

  • Commit changed from dcf608f261a4b2efa8e1f5195713e6a676366c07 to dc0cacbb78955e11376ec52885516d63dc6cb675

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

dc0cacbTrac 16158: add doctest to AffineScheme.__setstate__()

comment:7 Changed 7 years ago by git

  • Commit changed from dc0cacbb78955e11376ec52885516d63dc6cb675 to 8a65895c465ef8adb71bfad1048bc3d30f207994

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

88f7099Merge branch 'develop' into ticket/16158-Spec_functor
8a65895Trac 16158: fix capitalisation in "Category of schemes"

comment:8 Changed 7 years ago by tscrim

  • Branch changed from u/pbruin/16158-Spec_functor to u/tscrim/spec_functor-16158
  • Commit changed from 8a65895c465ef8adb71bfad1048bc3d30f207994 to 2d4dbfba46d6ab0859006185ddb14fa07dbc7a02
  • Reviewers set to Travis Scrimshaw

LGTM modulo some minor review tweaks I did. If you're happy with my changes, then positive review.


New commits:

753aa02Merge branch 'u/pbruin/16158-Spec_functor' of trac.sagemath.org:sage into u/pbruin/16158-Spec_functor
2d4dbfbSome minor review tweaks.

comment:9 Changed 7 years ago by git

  • Commit changed from 2d4dbfba46d6ab0859006185ddb14fa07dbc7a02 to f86c030c8e9f85dca352af3a25b636ea8b0b234e

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

f86c030Added one other # indirect doctest.

comment:10 Changed 7 years ago by pbruin

  • Branch changed from u/tscrim/spec_functor-16158 to u/pbruin/16158-Spec_functor
  • Commit changed from f86c030c8e9f85dca352af3a25b636ea8b0b234e to 185b49cd73907d2214401f96058e61b0eb334d8e
  • Status changed from needs_review to positive_review

Just some minor changes, thanks for the review.

comment:11 Changed 7 years ago by vbraun

  • Status changed from positive_review to needs_work
sage -t --long src/sage/schemes/elliptic_curves/ell_finite_field.py
**********************************************************************
File "src/sage/schemes/elliptic_curves/ell_finite_field.py", line 829, in sage.schemes.elliptic_curves.ell_finite_field.EllipticCurve_finite_field.cardinality
Failed example:
    EllipticCurve(GF(10007),[1,2,3,4,5]).cardinality(algorithm='foobar')
Expected:
    Traceback (most recent call last):
    ...
    ValueError: Algorithm is not known
Got:
    10076
**********************************************************************

comment:12 follow-up: Changed 7 years ago by pbruin

I am fairly sure that this is not caused by this ticket, but by #11474. The order is also computed in a doctest a few lines above; if the elliptic curve is still in the cache of the UniqueFactory from #11474, then it never looks at the algorithm parameter.

comment:13 follow-up: Changed 7 years ago by tscrim

  • Branch changed from u/pbruin/16158-Spec_functor to u/tscrim/spec_functor-16158
  • Commit changed from 185b49cd73907d2214401f96058e61b0eb334d8e to f62a54ac10f06b60e25a7efbfcf3157be00662d9
  • Status changed from needs_work to needs_review

Hmm...despite not making any changes to that file, it's somehow falling back to the cached version because it wasn't garbage collected. There doesn't seem to be a memory leak as far as I can tell, so I'm thinking it's just hanging around longer in the doctest. In either case, I made the check of the algorithm more robust.


New commits:

f62a54aMake the cardinality in ell_finite_field more robust.

comment:14 in reply to: ↑ 12 Changed 7 years ago by pbruin

Replying to pbruin:

I am fairly sure that this is not caused by this ticket, but by #11474. The order is also computed in a doctest a few lines above; if the elliptic curve is still in the cache of the UniqueFactory from #11474, then it never looks at the algorithm parameter.

This should be solved by #16680.

comment:15 in reply to: ↑ 13 Changed 7 years ago by pbruin

  • Branch changed from u/tscrim/spec_functor-16158 to u/pbruin/16158-Spec_functor
  • Commit changed from f62a54ac10f06b60e25a7efbfcf3157be00662d9 to 185b49cd73907d2214401f96058e61b0eb334d8e

Replying to tscrim:

Hmm...despite not making any changes to that file, it's somehow falling back to the cached version because it wasn't garbage collected. There doesn't seem to be a memory leak as far as I can tell, so I'm thinking it's just hanging around longer in the doctest. In either case, I made the check of the algorithm more robust.

Argh, didn't see this in time and made a different fix in #16680. I think it is a bit silly to check the algorithm keyword if we are going to return the cached version anyway. Maybe we can continue the discussion at #16680?

comment:16 follow-up: Changed 7 years ago by tscrim

Well then for dependencies, should we then have #11474 and #16680?

comment:17 in reply to: ↑ 16 ; follow-up: Changed 7 years ago by pbruin

Replying to tscrim:

Well then for dependencies, should we then have #11474 and #16680?

It seems to me that this ticket actually has nothing to do with the problem Volker found; it is #11474 that should have had #16680 as a dependency if it hadn't been closed already. Would you agree with setting this one back to positive review and finishing #16680 before the next beta?

comment:18 in reply to: ↑ 17 Changed 7 years ago by tscrim

Replying to pbruin:

It seems to me that this ticket actually has nothing to do with the problem Volker found; it is #11474 that should have had #16680 as a dependency if it hadn't been closed already. Would you agree with setting this one back to positive review and finishing #16680 before the next beta?

I can't reproduce the error Volker got with this ticket alone, so I agree with your assessment. Positive review and lets (quickly) finish #16680.

comment:19 Changed 7 years ago by vbraun

  • Dependencies changed from #15990, #16156 to #15990, #16156, #16680

comment:20 Changed 7 years ago by pbruin

  • Status changed from needs_review to positive_review

comment:21 Changed 7 years ago by vbraun

  • Branch changed from u/pbruin/16158-Spec_functor to 185b49cd73907d2214401f96058e61b0eb334d8e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.