#17425: Rational isometry test for quadratic forms over number fields
-------------------------------------+-------------------------------------
Reporter: annahaensch | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.9
Component: quadratic forms | Resolution:
Keywords: | Merged in:
Authors: Anna Haensch | Reviewers: Vincent Delecroix
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/annahaensch/ticket/17425 | 613901acd98fc0235f871336df3eaed5c6cf5a74
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Changes (by vdelecroix):
* status: needs_review => needs_work
* reviewer: => Vincent Delecroix
* milestone: sage-6.5 => sage-6.9
Comment:
Hello,
I am sorry that this ticket waited that long before a review.
1. There are some (minor) problems with typography:
- indentation is not correct. It should be
{{{
def my_function(a, b):
"""
The text here
...
"""
}}}
and not
{{{
def my_function(a, b):
"""
The text here
...
"""
}}}
- try to keep a space around the `==` and `!=` signs. In other words `a
== b` instead of `a==b`. Similarly, add space after a comma as
`function(a, b)` and not `function(a,b)`. This is just to clarify the
reading. It is not mandatory.
2. There is a method `.diagonal()` to extract the diagonal of a matrix
{{{
sage: K.<a> = NumberField(x^2-3)
sage: V = QuadraticForm(K,4,[1,0,0,0,2*a,0,0,a,0,2])
sage: m = V.Gram_matrix_rational()
sage: m.diagonal()
[1, 2*a, a, 2]
}}}
would be better than using the `map(lambda x:
M.Gram_matrix_rational()[x][x], ...)`
3. Instead of
{{{
if self.base_ring()==QQ:
return self.signature() == other.signature():
Rat_Isom_flag = False
}}}
you can do
{{{
if self.base_ring() == QQ:
return self.signature() == other.signature()
}}}
And more generally you do not need to carry over this variable
`Rat_Isom_flag`, just write some `return` in the conditional statements.
4. You can simplify the following
{{{
for i in range(len(K.real_embeddings())):
Mentries_emb = map(lambda(x):K.real_embeddings()[i](x),Mentries)
Nentries_emb = map(lambda(x):K.real_embeddings()[i](x),Nentries)
Mpos=0
for i in range(self.dim()):
if Mentries_emb[i]>=0:
Mpos += 1
...
}}}
into
{{{
for emb in K.real_embeddings():
Mpos = 0
for x in Mentries:
Mpos += emb(x) >= 0
...
}}}
I used two simplifications here
- you can iterate over a list elements
- adding a boolean to an integer is equivalent to adding `0` or `1`
depending on the truth value
Vincent
--
Ticket URL: <http://trac.sagemath.org/ticket/17425#comment:3>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
