sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.44k stars 480 forks source link

adding SchemeMorphism_points to coercion framework #23805

Closed bhutz closed 7 years ago

bhutz commented 7 years ago

This is part of meta ticket #23047. The goal is to have scheme points able to be coerced when that coercion makes sense. Essentially SchemeHomeset_points needs a _coerce_map_from_ function and possibly updates to _elementconstructor.

For example, we should be able to coerce between the following objects

Component: algebraic geometry

Author: Ben Hutz

Branch/Commit: 74347ea

Reviewer: Paul Fili, Adam Towsley

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

bhutz commented 7 years ago

Branch: u/bhutz/scheme_point_coercion

bhutz commented 7 years ago

Commit: c020db9

bhutz commented 7 years ago
comment:1

Here is a first implementation of coercion for scheme points. Comments welcome.


New commits:

c020db923805: coercion for scheme points
bhutz commented 7 years ago

Author: Ben Hutz

roed314 commented 7 years ago

Description changed:

--- 
+++ 
@@ -2,7 +2,7 @@

 For example, we should be able to coerce between the following objects

-- the same space type where the base rings have a coercion between them. i.e. P^1(ZZ) -> P^1(QQ)
+- the same space type where the base rings have a coercion between them. i.e. P<sup>1</sup>(ZZ) -> P<sup>1</sup>(QQ)

 - a points of a subscheme to points of the ambient space
roed314 commented 7 years ago
comment:3
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from c020db9 to 6d26bcc

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

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

6d26bcc23805: fixes from first review
bhutz commented 7 years ago
comment:5

Thanks for the comments. I've tried to better match what polynomial rings are doing. I don't think even a conversion from tuple/lists actually fits here very well. So I've no longer allowed that behind the scenes. i.e. being able to say P1([0,1]) == [0,1] does not work.

However, I feel out of my depth for Toric varieties. I'm not sure what type of situations it would actually be valid to coerce. Consequently, I taken to conservative approach and not done coercion for those. Would it be better to raise a NotImplementedError than just allowing _coerce_map_from_ to return None? or does this need to wait for someone who can deal with the Toric cases?

roed314 commented 7 years ago
comment:6

I agree that P1([0,1]) == [0,1] should not work, but again, make sure you keep coercion and conversion straight. The fact that P1([0,1]) works is a conversion, and is fine. the implicit construction described by the equality test you mention is a coercion, and is not.

As for toric varieties, it's fine leaving it as None. That just means that there is no coercion now; one can be added later if appropriate.

67c74138-1ca1-45a7-bae8-3eb17fc35cc7 commented 7 years ago
comment:8

I ran some functionality tests, the following examples should give true, but don't:

PQ.<x,y> = ProjectiveSpace(QQ,1)
PC.<x,y> = ProjectiveSpace(CC,1)
PQ([1,0])==PC([1,0]) , PC(CC).has_coerce_map_from(PQ(QQ))

and

R.<x> = PolynomialRing(QQ)
K.<i> = NumberField(x^2+1)
A1.<x> = AffineSpace(QQ,1)
A2.<x> = AffineSpace(K,1)
p1 = A1(1)
p2 = A2(1)
p1==p2 , A2(K).has_coerce_map_from(A1(QQ))
67c74138-1ca1-45a7-bae8-3eb17fc35cc7 commented 7 years ago
comment:9

Nevermind, those examples do work. My apologies.

pfili commented 7 years ago

Reviewer: Paul Fili

pfili commented 7 years ago
comment:10

I reviewed the code and I tried some rather sophisticated examples (couple of examples below), wasn't able to find any problems Looks good to me.

sage: R.<X> = PolynomialRing(QQ)
sage: K.<a> = NumberField(X^2 - 2, embedding=QQbar(sqrt(2)))
sage: P.<x,y> = ProjectiveSpace(QQbar, 1)
sage: Q.<x,y> = ProjectiveSpace(K, 1)
sage: P(sqrt(2),1) == Q(a,1)
sage: P(sqrt(2),1)
(1.414213562373095? : 1)
sage: Q(a, 1)
(a : 1)
sage: P(sqrt(2),1) == Q(a,1)
True
sage: P(QQbar).has_coerce_map_from(Q(K))
True

and

sage: R.<t> = PolynomialRing(QQ)
sage: K.<a> = NumberField(t^2 - 6, embedding=QQbar(sqrt(6)))
sage: P.<x,y,z,w> = ProjectiveSpace(QQbar, 3)
sage: X = P.subscheme(y^2*z - x*(x-z)*(x-w) ); X
Closed subscheme of Projective Space of dimension 3 over Algebraic Field defined by:
  -x^3 + x^2*z + y^2*z + x^2*w - x*z*w
sage: Y = P.subscheme([y^2*z - x*(x-z)*(x-w), w - 2*z]); Y
Closed subscheme of Projective Space of dimension 3 over Algebraic Field defined by:
  -x^3 + x^2*z + y^2*z + x^2*w - x*z*w,
  (-2)*z + w
sage: Y.change_ring(K)
Closed subscheme of Projective Space of dimension 3 over Number Field in a with defining polynomial t^2 - 6 defined by:
  -x^3 + x^2*z + y^2*z + x^2*w - x*z*w,
  -2*z + w
sage: P = Y([3,a,1,2]); P
(3/2 : 1.224744871391589? : 1/2 : 1)
sage: X(P)
verbose 0 (3370: multi_polynomial_ideal.py, groebner_basis) Warning: falling back to very slow toy implementation.
verbose 0 (3370: multi_polynomial_ideal.py, groebner_basis) Warning: falling back to very slow toy implementation.
verbose 0 (3370: multi_polynomial_ideal.py, groebner_basis) Warning: falling back to very slow toy implementation.
(3/2 : 1.224744871391589? : 1/2 : 1)
sage: Q = X([3,a,1,2])
sage: P == Q
verbose 0 (3370: multi_polynomial_ideal.py, groebner_basis) Warning: falling back to very slow toy implementation.
True
sage: Q == X(P) # should be true
True
sage: Q2 = X([3,-a,1,2])
sage: Q2 == P # should not be true
False
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

74347ea23805: add another subscheme example
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 6d26bcc to 74347ea

67c74138-1ca1-45a7-bae8-3eb17fc35cc7 commented 7 years ago
comment:12

It worked appropriately for every example that I came up with.

67c74138-1ca1-45a7-bae8-3eb17fc35cc7 commented 7 years ago

Changed reviewer from Paul Fili to Paul Fili, Adam Towsley

vbraun commented 7 years ago

Changed branch from u/bhutz/scheme_point_coercion to 74347ea