#12995 closed enhancement (fixed)
A first step towards linear systems of hypersurfaces in Sage
Reported by: | Moritz Minzlaff | Owned by: | Moritz Minzlaff |
---|---|---|---|
Priority: | major | Milestone: | sage-5.5 |
Component: | algebraic geometry | Keywords: | |
Cc: | Merged in: | sage-5.5.beta0 | |
Authors: | Moritz Minzlaff | Reviewers: | David Eklund |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
In Magma, one can do the following:
> Q := RationalField(); > P<x,y,z> := ProjectiveSpace(Q,2); > L := LinearSystem(P,2); > L; Linear system on Projective Space of dimension 2 Variables : x, y, z with 6 sections: x^2 x*y x*z y^2 y*z z^2 > p := P ! [3,2,1]; > L1 := LinearSystem(L,p); > L1; Linear system on Projective Space of dimension 2 Variables : x, y, z with 5 sections: x^2 - 9*z^2 x*y - 6*z^2 x*z - 3*z^2 y^2 - 4*z^2 y*z - 2*z^2
Sage does not have this functionality. This patch will be a first step towards adding a class LinearSystem? to Sage.
The goal is to add a method _linear_system_as_kernel to projective spaces that returns a matrix whose kernel can be identified with the degree d hypersurfaces with multiplicity at least m at pt.
(I actually need this method in the context of equimultiple liftings of plane curves over finite fields for which I will open a separate ticket.)
Apply:
Attachments (3)
Change History (29)
comment:1 Changed 10 years ago by
Status: | new → needs_review |
---|
comment:2 Changed 10 years ago by
Authors: | → Moritz Minzlaff |
---|
comment:3 Changed 10 years ago by
Reviewers: | → David Eklund |
---|
comment:4 Changed 10 years ago by
In the documentation part of the patch it says that the output is a matrix of size
{m-1+n choose n}
x{d+1 choose n}
but is the number of columns really {d+1 choose n}
and not {d+n choose n}
?
comment:5 Changed 10 years ago by
The syntax
raise Error, 'error message'
has been deprecated in Python. http://docs.python.org/release/2.6.8/tutorial/errors.html#raising-exceptions
We should use
raise Error('error message')
instead.
comment:7 Changed 10 years ago by
Dear David,
thanks for your remarks and your time to review this patch! I have uploaded a new version of the patch moments ago. You are quite right about the d+1 vs. d+n thing. I have also changed the syntax for raising Errors as you suggested.
Changed 10 years ago by
Attachment: | trac_12995_review.patch added |
---|
Fixed typos and line continuation of strings.
comment:9 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:10 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:11 Changed 10 years ago by
Hi!
I have a question: It says in the documentation part that the output is with respect to reverse lexicographic ordering of monomials. But it seems to me that, for example in the case of degree 2 in projective 2-space the monomials are ordered as:
[x^2,xy,xz,y^2,yz,z^2]
which seems to be just the lexicographic ordering rather than reverse lexicographic ordering.
If you agree I can incorporate this change into the review patch.
comment:12 Changed 10 years ago by
I don't have time tonight to answer your question, but I'll try to look at it tomorrow!
comment:13 Changed 10 years ago by
Ok, this looks good!
If you agree with the changes in the review patch and we resolve the issue about the monomial ordering, then we are good to go.
Changed 10 years ago by
Attachment: | trac_12995_review2.patch added |
---|
fixes two more things in documentation/comments
comment:14 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:15 Changed 10 years ago by
Hi David,
please excuse my slow response. I've recently left academia and can only find time to look at Sage in the evenings (but I am happy to do so!).
Thanks again for your comments. You are quite right, I've changed the documentation accordingly. I also saw where I got confused and changed one line of comment, too. :)
comment:16 Changed 10 years ago by
Thanks!
Actually, I think you are responding very quickly. I will try to get the patch bot to apply the right patches so that we can get rid of the ugly red blob at the top.
comment:17 Changed 10 years ago by
For the patchbot:
Apply trac_12995_initial.patch, trac_12995_review2.patch
comment:18 Changed 10 years ago by
Status: | needs_review → positive_review |
---|
comment:19 Changed 10 years ago by
Priority: | minor → major |
---|
comment:20 Changed 10 years ago by
Milestone: | sage-5.4 → sage-5.5 |
---|
comment:21 Changed 10 years ago by
Merged in: | → sage-5.5.beta0 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
comment:22 Changed 10 years ago by
Could you please make the commit message more descriptive than "incorporates David's remarks".
comment:23 Changed 10 years ago by
Yes, a more descriptive message would be better. Moritz, do you think you can take care of this? Thanks!
Changed 10 years ago by
Attachment: | trac_12995_initial.patch added |
---|
All tests pass on Sage 5.2 and Mac OS X lion (except two tests that fail on an unpatched Sage 5.2).
Let me look through the details and test the functionality and get back here.