Opened 11 years ago

Closed 9 years ago

#11863 closed enhancement (fixed)

Bilinear Map

Reported by: annahaensch Owned by: justin
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 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 annahaensch 11 years ago.
Trac_11863.patch (1.9 KB) - added by annahaensch 11 years ago.
Trac_11863v2.patch (2.1 KB) - added by annahaensch 10 years ago.
trac_11863_bilinear_form_v3.patch (2.8 KB) - added by chapoton 9 years ago.

Download all attachments as: .zip

Change History (22)

Changed 11 years ago by annahaensch

comment:1 Changed 11 years ago by annahaensch

  • Component changed from PLEASE CHANGE to quadratic forms
  • Owner changed from tbd to justin
  • Type changed from PLEASE CHANGE to enhancement

Changed 11 years ago by annahaensch

comment:2 Changed 11 years ago by annahaensch

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 knsam

  • Description modified (diff)
  • Status changed from new to needs_review

comment:4 Changed 10 years ago by knsam

  • Status changed from needs_review to needs_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 knsam (previous) (diff)

comment:5 Changed 10 years ago by knsam

  • Reviewers set to Kannappan Sampath

Changed 10 years ago by annahaensch

comment:6 follow-up: Changed 10 years ago by annahaensch

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 knsam

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 follow-up: Changed 9 years ago by 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 9 years ago by knsam

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 9 years ago by chapoton

comment:10 Changed 9 years ago by 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 follow-up: Changed 9 years ago by annahaensch

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 9 years ago by knsam

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 9 years ago by annahaensch

  • Authors set to Anna Haensch

comment:14 Changed 9 years ago by annahaensch

Kannappan, 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!

Last edited 9 years ago by annahaensch (previous) (diff)

comment:15 Changed 9 years ago by knsam

  • Authors changed from Anna Haensch to Anna Haensch, Frédéric Chapoton
  • Status changed from needs_info to needs_review

comment:16 Changed 9 years ago by knsam

  • Status changed from needs_review to positive_review

comment:17 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-5.8 to sage-5.9

comment:18 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.9.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.