Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#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:

Status badges

Description (last modified by Moritz Minzlaff)

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)

trac_12995_review.patch (1.8 KB) - added by David Eklund 10 years ago.
Fixed typos and line continuation of strings.
trac_12995_review2.patch (2.8 KB) - added by Moritz Minzlaff 10 years ago.
fixes two more things in documentation/comments
trac_12995_initial.patch (6.8 KB) - added by Moritz Minzlaff 10 years ago.

Download all attachments as: .zip

Change History (29)

comment:1 Changed 10 years ago by Moritz Minzlaff

Status: newneeds_review

comment:2 Changed 10 years ago by Moritz Minzlaff

Authors: Moritz Minzlaff

comment:3 Changed 10 years ago by David Eklund

Reviewers: David Eklund

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.

comment:4 Changed 10 years ago by David Eklund

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}?

Last edited 10 years ago by David Eklund (previous) (diff)

comment:5 Changed 10 years ago by David Eklund

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.

Last edited 10 years ago by David Eklund (previous) (diff)

comment:6 Changed 10 years ago by David Eklund

Tests pass on Sage 5.3 (Mac OS X lion) also.

comment:7 Changed 10 years ago by Moritz Minzlaff

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 10 years ago by David Eklund

Ok, looks good! I will put this to positive review soon.

Changed 10 years ago by David Eklund

Attachment: trac_12995_review.patch added

Fixed typos and line continuation of strings.

comment:9 Changed 10 years ago by David Eklund

Description: modified (diff)

comment:10 Changed 10 years ago by David Eklund

Description: modified (diff)

comment:11 Changed 10 years ago by David Eklund

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 Moritz Minzlaff

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 David Eklund

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 Moritz Minzlaff

Attachment: trac_12995_review2.patch added

fixes two more things in documentation/comments

comment:14 Changed 10 years ago by Moritz Minzlaff

Description: modified (diff)

comment:15 Changed 10 years ago by Moritz Minzlaff

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 David Eklund

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 David Eklund

For the patchbot:

Apply trac_12995_initial.patch, trac_12995_review2.patch

comment:18 Changed 10 years ago by David Eklund

Status: needs_reviewpositive_review

comment:19 Changed 10 years ago by David Eklund

Priority: minormajor

comment:20 Changed 10 years ago by Jeroen Demeyer

Milestone: sage-5.4sage-5.5

comment:21 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.5.beta0
Resolution: fixed
Status: positive_reviewclosed

comment:22 Changed 10 years ago by Jeroen Demeyer

Could you please make the commit message more descriptive than "incorporates David's remarks".

comment:23 Changed 10 years ago by David Eklund

Yes, a more descriptive message would be better. Moritz, do you think you can take care of this? Thanks!

comment:24 Changed 10 years ago by Moritz Minzlaff

Hi,

I will get to it later tonight/tomorrow.

Changed 10 years ago by Moritz Minzlaff

Attachment: trac_12995_initial.patch added

comment:25 Changed 10 years ago by Moritz Minzlaff

Ok, done. :)

comment:26 Changed 10 years ago by David Eklund

Thanks!

Note: See TracTickets for help on using tickets.