Opened 12 years ago

Closed 7 years ago

#7946 closed defect (fixed)

Fix TestSuite failures for schemes

Reported by: nthiery Owned by: AlexGhitza
Priority: major Milestone: sage-6.4
Component: algebraic geometry Keywords:
Cc: vbraun Merged in:
Authors: Peter Bruin Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 5fd92ee (Commits, GitHub, GitLab) Commit: 5fd92eed3024163fa068bea8e3644d677f151adc
Dependencies: #16158 Stopgaps:

Status badges

Description (last modified by pbruin)

Consider the following situation:

sage: S = Spec(ZZ)
sage: x = S.an_element()

Running TestSuite(S) and TestSuite(x) yields several failures. A related problem is

sage: S
Spectrum of Integer Ring
sage: parent(x)
Set of rational points of Spectrum of Integer Ring

whereas we expect parent(x) is S.

Here is the complete TestSuite report:

sage: TestSuite(S).run()
Failure in _test_an_element:
Traceback (most recent call last):
...
NotImplementedError
------------------------------------------------------------
  Failure in _test_category:
  Traceback (most recent call last):
  ...
  AssertionError: False is not true
  ------------------------------------------------------------
  Failure in _test_pickling:
  Traceback (most recent call last):
  ...
  AssertionError: Point on Spectrum of Integer Ring defined by the Principal ideal (991) of Integer Ring != Point on Spectrum of Integer Ring defined by the Principal ideal (991) of Integer Ring
  ------------------------------------------------------------
  The following tests failed: _test_category, _test_pickling
Failure in _test_elements
Failure in _test_some_elements:
Traceback (most recent call last):
...
NotImplementedError
------------------------------------------------------------
The following tests failed: _test_an_element, _test_elements, _test_some_elements
sage: TestSuite(x).run()
Failure in _test_category:
Traceback (most recent call last):
...
AssertionError: False is not true
------------------------------------------------------------
Failure in _test_pickling:
Traceback (most recent call last):
...
AssertionError: Point on Spectrum of Integer Ring defined by the Principal ideal (991) of Integer Ring != Point on Spectrum of Integer Ring defined by the Principal ideal (991) of Integer Ring
------------------------------------------------------------
The following tests failed: _test_category, _test_pickling

(Note: the NotImplementedError that one gets after applying #16158 is a different one than before.)

Change History (14)

comment:1 Changed 12 years ago by nthiery

  • Description modified (diff)

comment:2 Changed 12 years ago by AlexGhitza

  • Milestone set to sage-4.3.2

comment:3 Changed 10 years ago by novoselt

  • Cc vbraun added

#11599 claims to fix this, but I am not entirely sure what would it take to fix this ticket - the reported category is now schemes and only 3 of the TestSuite tests fail.

comment:4 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:5 Changed 8 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:6 Changed 7 years ago by pbruin

What is Spec intended to mean in the first place? The definition currently starts with

class Spec(AffineScheme):
    ....

This suggests that Spec is a special kind of affine scheme. But aren't "the spectrum of a ring" and "an affine scheme" exactly the same thing?

This code probably predates the category framework. A more modern design would perhaps be something like the following:

  • define the category AffineSchemes (or Schemes().Affine()) of affine schemes;
  • designate AffineScheme as the "canonical" type for objects in this category, and move functionality from Spec to AffineScheme as appropriate;
  • convert Spec into a functor from CommutativeRings to AffineSchemes.

(Mathematically speaking, Spec [as a functor from commutative rings to schemes, or even locally ringed spaces] can be defined as the adjoint functor of the global sections functor X -> OX(X) from schemes to commutative rings, and this gives an anti-equivalence of categories from commutative rings to affine schemes; I wonder if this could somehow be formalised in Sage's category framework, but that's another story...)

[Edit: the above idea is now essentially #16158, although there is no category of affine schemes yet.]

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

comment:7 Changed 7 years ago by pbruin

  • Dependencies set to #16158
  • Description modified (diff)
  • Summary changed from Spec(...) does not specify its category to Fix TestSuite failures for schemes

To show that the first part of the original ticket is fixed:

sage: S = Spec(ZZ)
sage: S.category()
Category of Schemes
sage: isinstance(S, S.category().parent_class) 
True

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

  • Authors set to Peter Bruin
  • Branch set to u/pbruin/7946-scheme_point_improvements
  • Commit set to 5fd92eed3024163fa068bea8e3644d677f151adc
  • Description modified (diff)
  • Status changed from new to needs_review

The attached branch makes affine schemes, and points on them, cooperate better with the category and coercion code. The TestSuite now runs without failures.

comment:9 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:10 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:11 in reply to: ↑ 8 ; follow-up: Changed 7 years ago by tscrim

  • Reviewers set to Travis Scrimshaw

Replying to pbruin:

The attached branch makes affine schemes, and points on them, cooperate better with the category and coercion code. The TestSuite now runs without failures.

Is there some way you can get rid of that custom __call__ method in AffineScheme, or at least passes things up through the Parent.__call__ when trying to construct an element? If not, then I think we're okay setting this to positive review as it fixes the issues as stated in the ticket description (you can do this on my behalf).

Also, I think that comment:6 (from the very limited understanding I have of the math) is the correct way to do things. However that would be a major overhaul to be done on a separate ticket, and perhaps the above change to __call__ would fit into that.

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

  • Status changed from needs_review to positive_review

Replying to tscrim:

Replying to pbruin:

The attached branch makes affine schemes, and points on them, cooperate better with the category and coercion code. The TestSuite now runs without failures.

Is there some way you can get rid of that custom __call__ method in AffineScheme, or at least passes things up through the Parent.__call__ when trying to construct an element? If not, then I think we're okay setting this to positive review as it fixes the issues as stated in the ticket description (you can do this on my behalf).

The reason this is done via the __call__() method is that an affine scheme S accepts two kinds of input; only one of them (input is a prime ideal) constructs an element with S as its parent. In the other case (input is a list of coordinates) the __call__() syntax constructs an element of a suitable scheme homset. I don't see an easy way to avoid a custom __call__() method; it may be possible (and would be nice) to get rid of it, but it is better to do this on separate ticket. (Note also that the __call__() method was already present, I just improved it as necessary.)

Also, I think that comment:6 (from the very limited understanding I have of the math) is the correct way to do things. However that would be a major overhaul to be done on a separate ticket, and perhaps the above change to __call__ would fit into that.

What I was talking about in that comment was mostly done on #16158 (closed a couple of months ago). I think the remaining issue of making a category AffineSchemes would probably be disjoint from getting rid of the __call__() method.

I'm setting this to positive review since you gave the green light.

comment:13 in reply to: ↑ 12 Changed 7 years ago by tscrim

The reason this is done via the __call__() method is that an affine scheme S accepts two kinds of input; only one of them (input is a prime ideal) constructs an element with S as its parent. In the other case (input is a list of coordinates) the __call__() syntax constructs an element of a suitable scheme homset. I don't see an easy way to avoid a custom __call__() method; it may be possible (and would be nice) to get rid of it, but it is better to do this on separate ticket. (Note also that the __call__() method was already present, I just improved it as necessary.)

That's what I was thinking from a quick look through. Anyways, for later.

What I was talking about in that comment was mostly done on #16158 (closed a couple of months ago). I think the remaining issue of making a category AffineSchemes would probably be disjoint from getting rid of the __call__() method.

I knew the last one was done, but forgot that it moved the functionality. Unfortunately I think you're right, but we can at least do a followup to create the category of AffineSchemes.

I'm setting this to positive review since you gave the green light.

Thanks.

comment:14 Changed 7 years ago by vbraun

  • Branch changed from u/pbruin/7946-scheme_point_improvements to 5fd92eed3024163fa068bea8e3644d677f151adc
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.