#21085 closed enhancement (fixed)
Blow ups and resolution of singularities for curves
Reported by:  gjorgenson  Owned by:  

Priority:  minor  Milestone:  sage7.4 
Component:  algebraic geometry  Keywords:  gsoc2016 
Cc:  bhutz, mmarco  Merged in:  
Authors:  Grayson Jorgenson  Reviewers:  Ben Hutz 
Report Upstream:  N/A  Work issues:  
Branch:  bebf7b0 (Commits)  Commit:  
Dependencies:  #21168, #21285  Stopgaps: 
Description (last modified by )
Implement functions to compute the blow up of an affine curve at a point, and to construct a nonsingular model for the curve by blowing up its singular points. These functions should describe the blow up/nonsingular model in terms of affine patches.
Change History (19)
comment:1 Changed 16 months ago by
 Branch set to u/gjorgenson/ticket/21085
comment:2 Changed 16 months ago by
 Commit set to f5fc535f7ca0a493f7e8b0b33079b8ffbc466fc1
comment:3 Changed 16 months ago by
 Commit changed from f5fc535f7ca0a493f7e8b0b33079b8ffbc466fc1 to 3dd26e7cd33ac155e9d102952701b26c2d6f5b99
Branch pushed to git repo; I updated commit sha1. New commits:
b923e36  21085: update the affine blow up function.

112b608  Merge branch 'master' into u/gjorgenson/ticket/21085

e4ea51e  21168: change_ring implementation

b80b533  21085: merge with ticket 21168 for access to change_ring function

3dd26e7  21085: resolution of singularities implementation, first attempt

comment:4 Changed 16 months ago by
 Dependencies set to #21168
 Description modified (diff)
 Milestone changed from sage7.3 to sage7.4
 Status changed from new to needs_review
 Summary changed from Blow ups for affine/projective curves to Blow ups and resolution of singularities for curves
comment:5 Changed 16 months ago by
 Commit changed from 3dd26e7cd33ac155e9d102952701b26c2d6f5b99 to 4a6bfbfe819f427ec1485b8b1e8ea0a704af6682
Branch pushed to git repo; I updated commit sha1. New commits:
4a6bfbf  21085: some minor changes

comment:6 Changed 16 months ago by
 Reviewers set to Ben Hutz
 Status changed from needs_review to needs_work
I think this is looking good overall. I have a few nits and a couple suggestions.
 you have no examples over
A^3
or higher even though it is in the general curve class
 I'm not sure I like the behavior that you get when you resolve a curve where all the singularities are not rational. You essentially get the curve itself back. Perhaps you should raise an error of the form "no rational singularities, use extend=True" or something like that.
 you should explicitly use the keyword in examples: 'extend=True' instead of just 'True'
 For the extend = True example, here is one that takes < 1s instead of the 3s that yours takes. I didn't look closely at yours, so if it tests additional functionality, then by all means leave it.
C=A.curve(x^4 + 2*x^2 + y^3 + 1)
 I was testing blowup() and really wanted to pass it a point other than the original. Is that a pain, to translate the point and then translate back?
 the error cases are not run in the examples and they should be
 in extension(): two code improvements
L = [pt[j] for j in range(len(self.ambient_space().gens())) for pt in pts]}}
can be the simpler
L = [t for pt in pts for t in pt ]
and
K = number_field_elements_from_algebraics(L)[0] return [K, self.base_ring().embeddings(K)[0]]
can be
K,a,phi = ... return [K,phi]
since number_field_elements_from_algebraic returns the embedding it used, so you can return that instead of just the first element of embeddings.
comment:7 Changed 16 months ago by
 Commit changed from 4a6bfbfe819f427ec1485b8b1e8ea0a704af6682 to 78e1ce23310ece13ece4eec995b849e5fc95ccf4
Branch pushed to git repo; I updated commit sha1. New commits:
78e1ce2  21085: changes from review

comment:8 Changed 16 months ago by
 Status changed from needs_work to needs_review
Thanks! Made most of the changes.
There were two A^3
examples in blowup() and resolution_of_singularities() so I left those as is, but I could still add some more higher dim. examples (the output gets long quickly).
For the extension() function I tried using the composite_fields functionality that number fields have; I think this might be cleaner than the approach I was trying with passing the generator of a number field as a QQbar element to number_field_elements_from_algebraics along with the other QQbar numbers in order to get a bigger field.
I think I could get blowup() to work for arbitrary points. After moving a point to the origin and blowing up, I'm not sure how to transform back though as the blowup will live in a mixed product space. I think maybe just constructing the ideal for the blow up centered at a different point directly might work.
Given a point P = (p_1,...,p_n)
, I think the ideal of the blow up at that point would be generated by the polynomials (x_i  p_i)s_j  (x_j  p_j)s_i
where the x_i
are the coords for the affine component, and the s_i
are the coords of the projective component of the product space. I tried something like this in my first attempt at the blowup function, but was unsure whether it was the right construction. Do you think this gives the right ideal? It seems like it should still give the closure of the graph of the rational map defined (x_1,...,x_n) > (x_1  p_1 :... : x_n  p_n)
which I think is how the blow up at P
is defined.
comment:9 Changed 16 months ago by
THis new resolution functionality looks fine to me.
For the blowup, yes that should be the correct ideal to blow up not at the origin. That is almost certainly easier than translating.
comment:10 Changed 15 months ago by
 Commit changed from 78e1ce23310ece13ece4eec995b849e5fc95ccf4 to 1cbb355d5f51adf31d06de992798825cead6566b
Branch pushed to git repo; I updated commit sha1. New commits:
1cbb355  21085: generalized blowup() to work for any point on a given curve

comment:11 Changed 15 months ago by
 Commit changed from 1cbb355d5f51adf31d06de992798825cead6566b to 78fdd8169cf389b44ff87dcfd0712bc66338114b
comment:12 Changed 15 months ago by
Ok, I generalized the blowup() function to work for arbitrary points. This simplified some of the code in resolution_of_singularities since there's no longer a need to keep track of change of coordinate maps used to move points to the origin. However, I ran into a couple of issues when testing some higher dimension examples.
It seems that the way I was trying to isolate the strict transforms of the curves wrt the blow ups isn't always sufficient for higher dimension curves, and sometimes the resulting curves would be reducible even though the original curves were irreducible. To address this, the blowup() function now looks at the irreducible components of the transformed curve (after substituting the relations that hold in a particular chart of the blowup) and ignores any component that lies within the exceptional divisor.
In implementing the above I noticed another issue which occurs when the transform of the curve coincides with the exceptional divisor. An example of this is when blowing up the line x
at the origin; in one of the charts the transformed line and the exceptional divisor both seem to be defined by x
. If I understand correctly the strict transform is the closure of the preimage by the projection map of the line minus the origin, which I think should be empty in that chart. The output in the current implementation is:
A.<x,y> = AffineSpace(QQ, 2) C = A.curve([x]) C.blowup([0,0])[0] (Closed subscheme of Affine Space of dimension 2 over Rational Field defined by: 1, Affine Plane Curve over Rational Field defined by s0)
Should the strict transform be empty in the first chart, and if so, would it be better to leave that chart out of the output?
I also encountered an unrelated issue with applying change_ring to the transition maps when extending the base field. A simplified example of the problem is:
K.<a> = QuadraticField(1) A.<x,y> = AffineSpace(K, 2) H = End(A) phi = H([x/y, y]) emb = K.embeddings(QQbar)[0] phi.change_ring(emb)
which raises a TypeError
without any description. This appears to only be an issue when there are actual fraction field elements defining the morphism. The line that breaks is R = self.coordinate_ring().hom(R, T.ambient_space().coordinate_ring())
in the change_ring
function for the SchemeMorphism_polynomial class of generic/morphism.py. Is this actually a bug, or perhaps am I just misusing the functionality?
comment:13 Changed 15 months ago by
Yes, that is a bug. In that case the coordinate ring of the map was a fraction field, so it failed to create the hom. It is now fixed in #21285, which needs review.
comment:14 Changed 15 months ago by
 Status changed from needs_review to needs_work
This is getting a little beyond my expertise, but what are describing sounds correct to me. As for what to do with the empty chart. I would prefer to return it so that there are 'right' number of charts. It is clearly empty based on the defining equations.
For the blowup at points. A couple things. While it is fine to accept a list as input and do the coercision in the function, I think it would be better to pass in actual points in the examples, maybe a curve point for one and an affine space point for another. Also, I'd like to see it default to the origin, so that the point parameter is optional.
comment:15 Changed 15 months ago by
 Commit changed from 78fdd8169cf389b44ff87dcfd0712bc66338114b to bebf7b02f5cbe21fe85975821414b78fa65efadc
comment:16 Changed 15 months ago by
 Dependencies changed from #21168 to #21168, #21285
 Status changed from needs_work to needs_review
Alright, I made the changes and did some more functionality testing. So far everything seems to be working as intended on the examples I've come up with, and I think this should be good for review again.
I've also been trying to implement a projective blow up function; for a projective curve defined by f_1,...,f_m
in P^n
, I was thinking to use the ideal generated by f_1,...,f_m, x_i*y_j  x_j*y_i
for i,j = 0,...,n1
in the product space P^n\times P^{n1}
to define the blow up of the curve at the point (0:...:0:1)
, where the x_i
are the coords for P^n
and the y_i
are the coords for P^{n1}
. However I'm not sure this is actually the correct ideal, and think it might be better to leave implementing projective blow ups at points for when more general functionality for blowing up along subschemes is implemented.
comment:17 Changed 15 months ago by
 Status changed from needs_review to positive_review
This seems fine to me, so I'll mark it positive.
As for the projective blowup. Those equations don't really make a lot of sense. I was trying to think of a general form for a projective blowup, but really, this is a local object and the most natural way to think of it is through affine charts anyway.
So I'm fine with holding off on projective blowups.
comment:18 Changed 15 months ago by
 Branch changed from u/gjorgenson/ticket/21085 to bebf7b02f5cbe21fe85975821414b78fa65efadc
 Resolution set to fixed
 Status changed from positive_review to closed
comment:19 Changed 15 months ago by
 Commit bebf7b02f5cbe21fe85975821414b78fa65efadc deleted
Any chance some of you have a look at https://trac.sagemath.org/ticket/17254#comment:364 ? With the Singular upgrade a few signs changed in the doctests from here and we have no idea if the result is still valid (although it does not look obviously false).
Branch pushed to git repo; I updated commit sha1. New commits:
21085: blow ups for affine curves