Change ring to QQbar fails for subschemes

[bhutz]

Change ring for subschemes does not let the user specify an embedding and so fails for QQbar

K.<w>=QuadraticField(2)
P.<x,y> = ProjectiveSpace(K,1)
X = P.subscheme(x-y)
X.change_ring(QQbar)

K.<w>=QuadraticField(2)
P.<x,y> = ProjectiveSpace(K,1)
X = P.subscheme(x-y)
H = End(X)
f = H([6*x^2+2*x*y+16*y^2,-3*x^2-4*x*y-4*y^2])
f.change_ring(QQbar)

Component: algebraic geometry

Author: Ben Hutz

Branch: e73cd1f

Reviewer: Rebecca Lauren Miller, Joseph Eisner

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

[bhutz]

Branch: u/bhutz/ticket/20067

[sagetrac-git]

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

276ff71

20067: improve change_ring() for subschemes

[sagetrac-git]

Commit: 276ff71

[bhutz]

comment:3

I added the embedding option for subschemes and made some other improvements to change_ring() in general to make it a little more robust.

[sagetrac-git]

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

6a39223

20067: try coercion first for change_ring

[sagetrac-git]

Changed commit from 276ff71 to 6a39223

[sagetrac-rlmiller]

Reviewer: Rebecca Miller, Joseph Eisner

[sagetrac-rlmiller]

Changed reviewer from Rebecca Miller, Joseph Eisner to Rebecca Lauren Miller, Joseph Eisner

[sagetrac-rlmiller]

comment:7

1. Has coerce map from instead of try accept
2. raise warning if embedding not specified and no coerce map

Still doing functionality testing.

[sagetrac-git]

Changed commit from 6a39223 to 05e47bb

[sagetrac-git]

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

05e47bb

20067: add warning when choosing embedding

[bhutz]

comment:9

changing the try/except to has_coerce_map_from reduced the functionality. For example QQ to GF(3) does not have a coerce map from, but can be done.

Consequently, I only added the warning.

[nbruin]

comment:10

Can't we just make sure that base_change or change_ring can also be given a ring homomorphism from the base ring, to base extend to the codomain of the homomorphism? Then

X=Scheme(Q,...)
k=GF(3)
Xk=X.base_change(k.convert_map_from(QQ))

would work (provided we don't check if the map really is a homomorphism).
Then we can have that X.base_change(GF(3)) can still raise an error (because it's not well-defined), but we can still get an answer.

[bhutz]

comment:11

Not sure exactly what you are suggesting here. If it is just the ability to supply the mapping, then that is actually the main change here. There is now an optional embedding parameter that can be specified

Xk=X_change_ring(embedding=k.convert_map_from(QQ))

The remaining issue was what to do if not specified. In this case, one is attempted to be constructed (the try/except parts...). When this is not unique, you seem to be suggesting raising an error and having the user construct the particular map they want. It seemed more user friendly to me to simply choose one and provide a warning that happened.

[nbruin]

comment:12

Replying to @bhutz:

The remaining issue was what to do if not specified. In this case, one is attempted to be constructed (the try/except parts...). When this is not unique, you seem to be suggesting raising an error and having the user construct the particular map they want. It seemed more user friendly to me to simply choose one and provide a warning that happened.

The problem is that we don't have a channel to reliably report warnings through. If you print them in places where people aren't particularly looking for output, they are easily missed. In fact, STDOUT might be redirected to /dev/null, and messages on STDERR might be hard to match up with the rest of the data (if it gets read at all).

To quote the Zen of Python: "In the face of ambiguity, refuse the temptation to guess.". I'm not for following the Zen religiously, but I think it's a good principle. In this case, I think the error is a more honest and ultimately more reliable solution than making some non-canonical guess.

[bhutz]

comment:13

I understand the concern and definitely didn't want to silently guess, hence the verbose warning. However, there was some precedent since

K.<w>=CyclotomicField(3)
CC(w)

just works and 'guesses' what embedding to use.

However, if just raising an error is more in-line with similar other functionality, I can do that instead.

[sagetrac-git]

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

e28d8be

20067: remove creation of embedding if not specified

[sagetrac-git]

Changed commit from 05e47bb to e28d8be

[bhutz]

comment:15

on further looking, that wasn't a precedent since cyclotomic fields create an embedding by default as part of their creation.

So here is a version where I've removed the 'guessing'

[sagetrac-git]

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

0882474

Merge branch 'master' into ticket/20067

[sagetrac-git]

Changed commit from e28d8be to 0882474

[bhutz]

comment:17

I don't like the embedding keyword syntax. I'm going to check this to accept passing the ring in. This is more compatible with the other existing change_ring functions.

[vbraun]

comment:21

Reviewer name is missing

[vbraun]

comment:22

I mean, Author name

[bhutz]

comment:23

I meant to set this to needs review. Added author.

[bhutz]

Author: Ben Hutz

[sagetrac-rlmiller]

comment:24

Looks good to me!

[vbraun]

comment:25

Merge conflict, please merge in the latest beta...

[sagetrac-git]

Changed commit from 9001f02 to e73cd1f

[sagetrac-git]

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

e73cd1f

Merge branch '7.1.beta6' into ticket/20067

[sagetrac-rlmiller]

comment:28

Looks good to me. No error with new commits in the latest beta.

[vbraun]

Changed branch from u/bhutz/ticket/20067 to e73cd1f

[jdemeyer]

comment:30

What do you prefer: "Rebecca Lauren Miller" or "Rebecca Miller"?

[jdemeyer]

Changed commit from e73cd1f to none

[kcrisman]

comment:31

Given that comment:6 was done by rlmiller, I would guess the "Lauren" version.

[sagetrac-rlmiller]

comment:32

Yes, The Lauren Version. I will update all previous tickets.