Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Spec into a functor #16158

Closed
pjbruin opened this issue Apr 14, 2014 · 35 comments
Closed

Make Spec into a functor #16158

pjbruin opened this issue Apr 14, 2014 · 35 comments

Comments

@pjbruin
Copy link
Contributor

pjbruin commented Apr 14, 2014

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.

Depends on #15990
Depends on #16156
Depends on #16680

CC: @nthiery @vbraun

Component: algebraic geometry

Keywords: Spec functor affine scheme

Author: Peter Bruin

Branch/Commit: 185b49c

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/16158

@pjbruin pjbruin added this to the sage-6.2 milestone Apr 14, 2014
@pjbruin
Copy link
Contributor Author

pjbruin commented Apr 14, 2014

Branch: u/pbruin/16158-Spec_functor

@pjbruin
Copy link
Contributor Author

pjbruin commented Apr 14, 2014

comment:1

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.]

@pjbruin
Copy link
Contributor Author

pjbruin commented Apr 14, 2014

Commit: 9d4ddc6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 15, 2014

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 15, 2014

Changed commit from 9d4ddc6 to cb82f01

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 23, 2014

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

dcf608fMerge branch 'develop' into ticket/16158-Spec_functor

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 23, 2014

Changed commit from cb82f01 to dcf608f

@pjbruin

This comment has been minimized.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2014

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

dc0cacbTrac 16158: add doctest to AffineScheme.__setstate__()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 10, 2014

Changed commit from dcf608f to dc0cacb

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 25, 2014

Changed commit from dc0cacb to 8a65895

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 25, 2014

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"

@tscrim
Copy link
Collaborator

tscrim commented Jul 18, 2014

Changed branch from u/pbruin/16158-Spec_functor to u/tscrim/spec_functor-16158

@tscrim
Copy link
Collaborator

tscrim commented Jul 18, 2014

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jul 18, 2014

Changed commit from 8a65895 to 2d4dbfb

@tscrim
Copy link
Collaborator

tscrim commented Jul 18, 2014

comment:8

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2014

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

f86c030Added one other # indirect doctest.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2014

Changed commit from 2d4dbfb to f86c030

@pjbruin
Copy link
Contributor Author

pjbruin commented Jul 18, 2014

Changed commit from f86c030 to 185b49c

@pjbruin
Copy link
Contributor Author

pjbruin commented Jul 18, 2014

comment:10

Just some minor changes, thanks for the review.

@pjbruin
Copy link
Contributor Author

pjbruin commented Jul 18, 2014

Changed branch from u/tscrim/spec_functor-16158 to u/pbruin/16158-Spec_functor

@vbraun
Copy link
Member

vbraun commented Jul 18, 2014

comment:11
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
**********************************************************************

@pjbruin
Copy link
Contributor Author

pjbruin commented Jul 18, 2014

comment:12

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.

@tscrim
Copy link
Collaborator

tscrim commented Jul 18, 2014

Changed commit from 185b49c to f62a54a

@tscrim
Copy link
Collaborator

tscrim commented Jul 18, 2014

Changed branch from u/pbruin/16158-Spec_functor to u/tscrim/spec_functor-16158

@tscrim
Copy link
Collaborator

tscrim commented Jul 18, 2014

comment:13

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.

@pjbruin
Copy link
Contributor Author

pjbruin commented Jul 18, 2014

comment:14

Replying to @pjbruin:

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.

@pjbruin
Copy link
Contributor Author

pjbruin commented Jul 18, 2014

Changed branch from u/tscrim/spec_functor-16158 to u/pbruin/16158-Spec_functor

@pjbruin
Copy link
Contributor Author

pjbruin commented Jul 18, 2014

Changed commit from f62a54a to 185b49c

@pjbruin
Copy link
Contributor Author

pjbruin commented Jul 18, 2014

comment:15

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?

@tscrim
Copy link
Collaborator

tscrim commented Jul 18, 2014

comment:16

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

@pjbruin
Copy link
Contributor Author

pjbruin commented Jul 19, 2014

comment:17

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?

@tscrim
Copy link
Collaborator

tscrim commented Jul 19, 2014

comment:18

Replying to @pjbruin:

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.

@vbraun
Copy link
Member

vbraun commented Jul 19, 2014

Changed dependencies from #15990, #16156 to #15990, #16156, #16680

@vbraun
Copy link
Member

vbraun commented Jul 20, 2014

Changed branch from u/pbruin/16158-Spec_functor to 185b49c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants