Opened 11 years ago

Closed 10 years ago

#11863 closed enhancement (fixed)

Bilinear Map

Reported by: Anna Haensch Owned by: Justin Walker
Priority: minor Milestone: sage-5.9
Component: quadratic forms Keywords:
Cc: Merged in: sage-5.9.beta0
Authors: Anna Haensch, Frédéric Chapoton Reviewers: Kannappan Sampath
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Frédéric Chapoton)

This function evaluates two vectors in a quadratic space under the bilinear map associated to the space.

This is sort of a trivial function, and it may exist already...I couldn't find it though. It is mostly important because it simplifies the implementation of other quadratic forms functions, such as Ticket #11841.

Apply trac_11863_bilinear_form_v3.patch

Attachments (4)

bilinear_map.patch (1.9 KB) - added by Anna Haensch 11 years ago.
Trac_11863.patch (1.9 KB) - added by Anna Haensch 11 years ago.
Trac_11863v2.patch (2.1 KB) - added by Anna Haensch 10 years ago.
trac_11863_bilinear_form_v3.patch (2.8 KB) - added by Frédéric Chapoton 10 years ago.

Download all attachments as: .zip

Change History (22)

Changed 11 years ago by Anna Haensch

Attachment: bilinear_map.patch added

comment:1 Changed 11 years ago by Anna Haensch

Component: PLEASE CHANGEquadratic forms
Owner: changed from tbd to Justin Walker
Type: PLEASE CHANGEenhancement

Changed 11 years ago by Anna Haensch

Attachment: Trac_11863.patch added

comment:2 Changed 11 years ago by Anna Haensch

Trac_11863.patch is the same as bilinear_map.patch, I just wanted to stick to the naming convention. Sorry about that.

comment:3 Changed 10 years ago by Kannappan Sampath

Description: modified (diff)
Status: newneeds_review

comment:4 Changed 10 years ago by Kannappan Sampath

Status: needs_reviewneeds_info

AFAI looked, this is not implemented. And, indeed, it would be a very useful addition for implementing a lot of other things. But, I do have a couple of comments:

  1. I am OK with this method being implemented in quadratic_forms.py. But, do people think that, this method should be added to the Cython-based quadratic_form__evaluate.pyx? (I am guessing based on the name of the file.)

About the patch:

  1. The docstring now reads: "Given a quadratic form Q', this gives the image of two vectors in Q` under the associated bilinear map."
  • As you can see the docstring formatting is messed up: The first Q should be enclosed by backticks on either side as opposed to a single quote. Sorry about the minor nitpick.
  • The vectors are in the base ring and not "in Q". A minor typo here. :)
  • Could you please define what the "associated bilinear form" is?
  • From the implementation, I think we don't want the ring to be of characteristic 2, as 1/2 would not make sense in such a ring. How are we going to take care of this? (ring has a characteristic() method...)
  1. Could you please add a space between the docstring and the INPUTS?

Cheers, Kannappan.

Last edited 10 years ago by Kannappan Sampath (previous) (diff)

comment:5 Changed 10 years ago by Kannappan Sampath

Reviewers: Kannappan Sampath

Changed 10 years ago by Anna Haensch

Attachment: Trac_11863v2.patch added

comment:6 Changed 10 years ago by Anna Haensch

Thanks for the comments Kannappan!

I fixed up the errors in the documentation, thanks for spotting that characteristic 2 error, that's an important one. I chose to leave the function in the quadratic_forms.py folder.

Perhaps you could run some doctests and coverage tests for me if you get the chance...or maybe you already did?

Best, Anna

comment:7 in reply to:  6 Changed 10 years ago by Kannappan Sampath

Replying to annahaensch: Hello Anna!

Thanks for the comments Kannappan!

I fixed up the errors in the documentation, thanks for spotting that characteristic 2 error, that's an important one. I chose to leave the function in the quadratic_forms.py folder.

Looks better.

  • The vectors are still from Q but their entries should be really from the base ring of Q (they must come from base_ring^no_of_variables).
  • I just have one more question: most books on Quadratic forms (eg. Scharlau's) seem to prefer without dividing by 2. Do you think we must really divide by 2?
    • And, they also call this the polar form corresponding to the given quadratic form. So, how about an alias: polar_form?
  • Also, given a bilinear form, we could get a quadratic form in a canonical way. Is this implemented?

Apart from this, we could set this to positive review.

Perhaps you could run some doctests and coverage tests for me if you get the chance...or maybe you already did?

I did! I have come up with a couple of issues that we need to fix.

  • First, the look of the quadratic_form.html - I shall open a ticket for this.
  • Second, there is a failing doctest in equivalence testing: #14093.

Could you please review this?

  • Thirdly, the file automorphisms seems to have a couple of issues, I have not yet worked out. I have posted on sage-devel. Please do look. You may have some comments.

~KnS

comment:8 Changed 10 years ago by Frédéric Chapoton

You should raise TypeError? and not return them.

raise TypeError("Bilinear form can only be evaluated on vectors with length " + str(self.dim()) + ".")

and

raise TypeError("Not defined for rings of characteristic 2.")

There should also be a doctest for the second raise error.

Please look at http://www.sagemath.org/doc/developer/conventions.html for the syntax of INPUT and OUTPUT fields (no indentation needed)

comment:9 in reply to:  8 Changed 10 years ago by Kannappan Sampath

Replying to chapoton:

Hello Chapoton,

raise TypeError("Not defined for rings of characteristic 2.")

The Whole point of my comments in the previous message, was whether we should divide by 2 at all. Scharlau, for example does not seem to require that.

Changed 10 years ago by Frédéric Chapoton

comment:10 Changed 10 years ago by Frédéric Chapoton

Description: modified (diff)

Here is a clean patch, to use as new starting point for further discussions. There still remains the question about 2.

For the bot:

apply trac_11863_bilinear_form_v3.patch

comment:11 Changed 10 years ago by Anna Haensch

Chapoton, thank you for the updated patch, that looks quite good.

Regarding the question about 2, I am following the conventions set forth in O'meara, for which we require a field of characteristic not 2. This is different from the conventions used in Scharlau's work, but is equally standard.

Also, given a bilinear form, we could get a quadratic form in a canonical way. 
Is this implemented?

Yes, we can! I'm not sure if it's implemented. Certainly it would be an issue separate from this function, since this uses the quadratic form to define the bilinear. But that is something to consider in the future.

comment:12 in reply to:  11 Changed 10 years ago by Kannappan Sampath

Replying to annahaensch:

Hello Anna,

Regarding the question about 2, I am following the conventions set forth in O'meara, for which we require a field of characteristic not 2. This is different from the conventions used in Scharlau's work, but is equally standard.

Anna, I sincerely hope you're not offended! I am absolutely fine with both of these conventions. I am convinced that this should be given positive review now! If you agree (could chapoton and you please fill in your real name as author), I shall set this to positive review.

Yes, we can! I'm not sure if it's implemented. Certainly it would be an issue separate from this function, since this uses the quadratic form to define the bilinear. But that is something to consider in the future.

Agreed! I shall open a new ticket.

comment:13 Changed 10 years ago by Anna Haensch

Authors: Anna Haensch

comment:14 Changed 10 years ago by Anna Haensch

Knsam, I'm not the least bit offended :) I sincerely appreciate your help and input on this patch. I feel like a positive review would be very good at this point, please proceed with that. Thanks!

Version 0, edited 10 years ago by Anna Haensch (next)

comment:15 Changed 10 years ago by Kannappan Sampath

Authors: Anna HaenschAnna Haensch, Frédéric Chapoton
Status: needs_infoneeds_review

comment:16 Changed 10 years ago by Kannappan Sampath

Status: needs_reviewpositive_review

comment:17 Changed 10 years ago by Jeroen Demeyer

Milestone: sage-5.8sage-5.9

comment:18 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.9.beta0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.