Opened 3 years ago

Closed 3 years ago

#29145 closed enhancement (fixed)

Indeterminacy locus and image of morphisms of schemes

Reported by: klee Owned by:
Priority: minor Milestone: sage-9.1
Component: algebraic geometry Keywords:
Cc: Merged in:
Authors: Kwankyu Lee Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 7ad46c0 (Commits, GitHub, GitLab) Commit: 7ad46c06e157a9fdc464405630c1b110de9a90b4
Dependencies: Stopgaps:

Status badges

Description (last modified by klee)

Implemented indeterminacy locus and image of morphisms of affine and projective schemes.

Along the way, cleaned up some files in schemes module.


The author was supported by NRF of Korea 2019, 2020

Change History (16)

comment:1 Changed 3 years ago by klee

Branch: u/klee/29145

comment:2 Changed 3 years ago by git

Commit: 17aec4e47066e9ec0ebfa9a39086f07d6daa5eee

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

17aec4eAdded indeterminacy_locus() and image()

comment:3 Changed 3 years ago by klee

Status: newneeds_review

comment:4 Changed 3 years ago by klee

Authors: Kwankyu Lee

comment:5 Changed 3 years ago by klee

Description: modified (diff)

comment:6 Changed 3 years ago by git

Commit: 17aec4e47066e9ec0ebfa9a39086f07d6daa5eee721b23ecf740dc6fd6f14b3b1049d881ad58fec4

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

721b23eFixes for doctest failures

comment:7 Changed 3 years ago by git

Commit: 721b23ecf740dc6fd6f14b3b1049d881ad58fec45c49204281bce6053fa52b3187c72b610aee74f9

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

5c49204Added indeterminacy_locus() and image()

comment:8 Changed 3 years ago by git

Commit: 5c49204281bce6053fa52b3187c72b610aee74f9aded9e3da7e46de08ac92b90efa0a3af58ac5cf9

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

aded9e3Added a reference

comment:9 Changed 3 years ago by git

Commit: aded9e3da7e46de08ac92b90efa0a3af58ac5cf94ea25f2759580b3bfabbc4a3c02eb2981bc3d8fb

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

4ea25f2A pyflakes fix

comment:10 Changed 3 years ago by git

Commit: 4ea25f2759580b3bfabbc4a3c02eb2981bc3d8fb383b618a6400b1f69a3c447e6536841c9143280e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

383b618Added indeterminacy_locus() and image()

comment:11 Changed 3 years ago by tscrim

A few things:

It would be good to have a more clear indication for this list here

             sage: g.representatives()
             [Scheme morphism:
                From: Closed subscheme of Affine Space of dimension 2 over Rational Field defined by:
                x^2 - y^2 - y
                To:   Affine Space of dimension 1 over Rational Field
                Defn: Defined on coordinates by sending (x, y) to
-                     (x/y), Scheme morphism:
+                     (x/y),
+             Scheme morphism:
                From: Closed subscheme of Affine Space of dimension 2 over Rational Field defined by:
                x^2 - y^2 - y
                To:   Affine Space of dimension 1 over Rational Field
                Defn: Defined on coordinates by sending (x, y) to
                      ((y + 1)/x)]

and similar changes for the other tests in this method.

Do you need to move the SchemeMorphism.__init__(self, parent) in SchemeMorphism_polynomial.__init__? I feel it would be better to do it after the possible error messages for when those are raised so as to not do unnecessary work (granted, if it is needed to be initialized for the tests, then this comment is moot).

PEP8:

-p = source_ring(poly.numerator())/source_ring(poly.denominator())
+p = source_ring(poly.numerator()) / source_ring(poly.denominator())

I don't understand this bit of code:

        if not self.codomain().is_projective():
            pass

There is no else statement attached, so I feel it can be removed.

Is the Chow_form still usable by instances of AlgebraicScheme_subscheme_projective? Are they suppose to be?

comment:12 in reply to:  11 Changed 3 years ago by klee

Replying to tscrim:

A few things:

It would be good to have a more clear indication for this list here

             sage: g.representatives()
             [Scheme morphism:
                From: Closed subscheme of Affine Space of dimension 2 over Rational Field defined by:
                x^2 - y^2 - y
                To:   Affine Space of dimension 1 over Rational Field
                Defn: Defined on coordinates by sending (x, y) to
-                     (x/y), Scheme morphism:
+                     (x/y),
+             Scheme morphism:
                From: Closed subscheme of Affine Space of dimension 2 over Rational Field defined by:
                x^2 - y^2 - y
                To:   Affine Space of dimension 1 over Rational Field
                Defn: Defined on coordinates by sending (x, y) to
                      ((y + 1)/x)]

and similar changes for the other tests in this method.

OK. Done.

Do you need to move the SchemeMorphism.__init__(self, parent) in SchemeMorphism_polynomial.__init__? I feel it would be better to do it after the possible error messages for when those are raised so as to not do unnecessary work (granted, if it is needed to be initialized for the tests, then this comment is moot).

No. You are right. I wrongly thought it is needed. Moved back.

PEP8:

-p = source_ring(poly.numerator())/source_ring(poly.denominator())
+p = source_ring(poly.numerator()) / source_ring(poly.denominator())

Done.

I don't understand this bit of code:

        if not self.codomain().is_projective():
            pass

There is no else statement attached, so I feel it can be removed.

This part was "to do", and then I somehow forgot.. Now I filled the missing part, which is for morphisms from projective subscheme to affine space. Thank you for spotting this!

Is the Chow_form still usable by instances of AlgebraicScheme_subscheme_projective? Are they suppose to be?

Chow_form method assumes the base ring is a field. Methods of AlgebraicScheme_subscheme_projective should assume just rings. So it would be usable when the assumption is met.

comment:13 Changed 3 years ago by git

Commit: 383b618a6400b1f69a3c447e6536841c9143280e213ff8ffabd942b37aab704371ceb1695629ea67

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

213ff8fFixes for reviewer comments

comment:14 Changed 3 years ago by git

Commit: 213ff8ffabd942b37aab704371ceb1695629ea677ad46c06e157a9fdc464405630c1b110de9a90b4

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

7ad46c0Fixes for reviewer comments

comment:15 Changed 3 years ago by tscrim

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

Thank you for the changes and explanations. LGTM.

comment:16 Changed 3 years ago by vbraun

Branch: u/klee/291457ad46c06e157a9fdc464405630c1b110de9a90b4
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.