#12995 closed enhancement (fixed)
A first step towards linear systems of hypersurfaces in Sage
Reported by: | minz | Owned by: | minz |
---|---|---|---|
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 8 years ago by
- Status changed from new to needs_review
comment:2 Changed 8 years ago by
comment:3 Changed 7 years ago by
- Reviewers set to David Eklund
comment:4 Changed 7 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 7 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:6 Changed 7 years ago by
Tests pass on Sage 5.3 (Mac OS X lion) also.
comment:7 Changed 7 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.
comment:8 Changed 7 years ago by
Ok, looks good! I will put this to positive review soon.
comment:9 Changed 7 years ago by
- Description modified (diff)
comment:10 Changed 7 years ago by
- Description modified (diff)
comment:11 Changed 7 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 7 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 7 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.
comment:14 Changed 7 years ago by
- Description modified (diff)
comment:15 Changed 7 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 7 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 7 years ago by
For the patchbot:
Apply trac_12995_initial.patch, trac_12995_review2.patch
comment:18 Changed 7 years ago by
- Status changed from needs_review to positive_review
comment:19 Changed 7 years ago by
- Priority changed from minor to major
comment:20 Changed 7 years ago by
- Milestone changed from sage-5.4 to sage-5.5
comment:21 Changed 7 years ago by
- Merged in set to sage-5.5.beta0
- Resolution set to fixed
- Status changed from positive_review to closed
comment:22 Changed 7 years ago by
Could you please make the commit message more descriptive than "incorporates David's remarks".
comment:23 Changed 7 years ago by
Yes, a more descriptive message would be better. Moritz, do you think you can take care of this? Thanks!
comment:24 Changed 7 years ago by
Hi,
I will get to it later tonight/tomorrow.
Changed 7 years ago by
comment:25 Changed 7 years ago by
Ok, done. :)
comment:26 Changed 7 years ago by
Thanks!
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.