Opened 11 years ago
Last modified 10 years ago
#11863 closed enhancement
Bilinear Map — at Version 10
Reported by: | Anna Haensch | Owned by: | Justin Walker |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.9 |
Component: | quadratic forms | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | Kannappan Sampath | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
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.
Change History (14)
Changed 11 years ago by
Attachment: | bilinear_map.patch added |
---|
comment:1 Changed 11 years ago by
Component: | PLEASE CHANGE → quadratic forms |
---|---|
Owner: | changed from tbd to Justin Walker |
Type: | PLEASE CHANGE → enhancement |
Changed 11 years ago by
Attachment: | Trac_11863.patch added |
---|
comment:2 Changed 11 years ago by
comment:3 Changed 10 years ago by
Description: | modified (diff) |
---|---|
Status: | new → needs_review |
comment:4 Changed 10 years ago by
Status: | needs_review → 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:
- 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-basedquadratic_form__evaluate.pyx
? (I am guessing based on the name of the file.)
About the patch:
- 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 acharacteristic()
method...)
- Could you please add a space between the docstring and the INPUTS?
Cheers, Kannappan.
comment:5 Changed 10 years ago by
Reviewers: | → Kannappan Sampath |
---|
Changed 10 years ago by
Attachment: | Trac_11863v2.patch added |
---|
comment:6 follow-up: 7 Changed 10 years ago by
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 Changed 10 years ago by
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 ofQ
(they must come frombase_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
?
- And, they also call this the polar form corresponding to the given quadratic form. So, how about an alias:
- 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: 9 Changed 10 years ago by
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 Changed 10 years ago by
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
Attachment: | trac_11863_bilinear_form_v3.patch added |
---|
comment:10 Changed 10 years ago by
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
Trac_11863.patch is the same as bilinear_map.patch, I just wanted to stick to the naming convention. Sorry about that.