Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A first step towards linear systems of hypersurfaces in Sage #12995

Closed
mminzlaff mannequin opened this issue May 23, 2012 · 26 comments
Closed

A first step towards linear systems of hypersurfaces in Sage #12995

mminzlaff mannequin opened this issue May 23, 2012 · 26 comments

Comments

@mminzlaff
Copy link
Mannequin

mminzlaff mannequin commented May 23, 2012

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:

Component: algebraic geometry

Author: Moritz Minzlaff

Reviewer: David Eklund

Merged: sage-5.5.beta0

Issue created by migration from https://trac.sagemath.org/ticket/12995

@mminzlaff mminzlaff mannequin added this to the sage-5.4 milestone May 23, 2012
@mminzlaff mminzlaff mannequin self-assigned this May 23, 2012
@mminzlaff mminzlaff mannequin added the s: needs review label May 23, 2012
@mminzlaff
Copy link
Mannequin Author

mminzlaff mannequin commented May 23, 2012

Author: Moritz Minzlaff

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Aug 23, 2012

Reviewer: David Eklund

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Aug 23, 2012

comment:3

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.

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Aug 23, 2012

comment:4

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

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Sep 19, 2012

comment:5

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.

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Sep 19, 2012

comment:6

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

@mminzlaff
Copy link
Mannequin Author

mminzlaff mannequin commented Sep 19, 2012

comment:7

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.

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Sep 20, 2012

comment:8

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

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Sep 23, 2012

Attachment: trac_12995_review.patch.gz

Fixed typos and line continuation of strings.

@sagetrac-davideklund

This comment has been minimized.

@sagetrac-davideklund

This comment has been minimized.

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Sep 26, 2012

comment:11

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<sup>2,xy,xz,y</sup>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.

@mminzlaff
Copy link
Mannequin Author

mminzlaff mannequin commented Sep 26, 2012

comment:12

I don't have time tonight to answer your question, but I'll try to look at it tomorrow!

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Sep 28, 2012

comment:13

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.

@mminzlaff
Copy link
Mannequin Author

mminzlaff mannequin commented Sep 29, 2012

Attachment: trac_12995_review2.patch.gz

fixes two more things in documentation/comments

@mminzlaff

This comment has been minimized.

@mminzlaff
Copy link
Mannequin Author

mminzlaff mannequin commented Sep 29, 2012

comment:15

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. :)

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Sep 29, 2012

comment:16

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.

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Sep 29, 2012

comment:17

For the patchbot:

Apply trac_12995_initial.patch, trac_12995_review2.patch

@jdemeyer jdemeyer removed this from the sage-5.4 milestone Sep 30, 2012
@jdemeyer jdemeyer added this to the sage-5.5 milestone Sep 30, 2012
@jdemeyer
Copy link
Contributor

Merged: sage-5.5.beta0

@jdemeyer
Copy link
Contributor

comment:22

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

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Oct 26, 2012

comment:23

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

@mminzlaff
Copy link
Mannequin Author

mminzlaff mannequin commented Oct 27, 2012

comment:24

Hi,

I will get to it later tonight/tomorrow.

@mminzlaff
Copy link
Mannequin Author

mminzlaff mannequin commented Oct 29, 2012

Attachment: trac_12995_initial.patch.gz

@mminzlaff
Copy link
Mannequin Author

mminzlaff mannequin commented Oct 29, 2012

comment:25

Ok, done. :)

@sagetrac-davideklund
Copy link
Mannequin

sagetrac-davideklund mannequin commented Oct 29, 2012

comment:26

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant