#18505: Enhance matrix similarity check
-------------------------------------+-------------------------------------
Reporter: rbeezer | Owner:
Type: enhancement | Status: needs_work
Priority: minor | Milestone: sage-6.9
Component: linear algebra | Resolution:
Keywords: matrix is_similar | Merged in:
similarity | Reviewers: Vincent Delecroix
Authors: Rob Beezer | Work issues:
Report Upstream: N/A | Commit:
Branch: u/rbeezer/is- | 465e2287bbe34ddcfe4190799e33b5ccf5ec980a
similar | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Changes (by vdelecroix):
* status: needs_review => needs_work
* reviewer: => Vincent Delecroix
* milestone: sage-6.8 => sage-6.9
Comment:
Hello,
This looks like a very nice improvement! I am slowly digging into the
algorithmic part of it. But let me first start with simple remarks.
1. First of all, the standard for doctest continuation is now `....:`
instead of `...`. Moreover, when `transformation=True` and when the
matrices are similar the method `is_similar` used to return `(False,
matrix)` instead of `(True, matrix)`. You are carefully ignoring the first
argument in all the tests! So either it is useless and I suggest to just
return `None` or `matrix` if `transformation=True` or test the first
argument appropriately. Finally, some tiny improvements can be obtained
using `m.get_unsafe(i,j)` instead of `m[i,j]`. I provided 3 commits for
these that can be found at `public/18505`
2. Instead as invoking QQbar if the ground field is QQ you can use the
method `.algebraic_closure()` that also works for prime fields
{{{
sage: QQ.algebraic_closure()
Algebraic Field
sage: GF(3).algebraic_closure()
Algebraic closure of Finite Field of size 3
}}}
Though, I do not know whether your code would work in this case.
3. I do not like the fact that for input in ZZ, the transformation matrix
has coefficient in QQbar. I do not see why the base field should be
extended. Conjugacy makes sense within `M_n(ZZ)`.
{{{
sage: m = matrix(ZZ,2,[0,1,1,1])
sage: a = matrix(ZZ,2,[1,1,0,1])
sage: b = matrix(ZZ,2,[1,0,1,1])
sage: c = a*b*b*a*~b
sage: m2 = c * m * ~c
sage: m.is_similar(m2)
True
sage: m.is_similar(m2, True)
(
[ 1.000000000000000? 0.?e-18]
True, [ 0.7894736842105263? -0.05263157894736842?]
)
}}}
(sided note: the above command used to return `(False, matrix)`... see
item 0)
4. In the case matrices are not square or have different dimension I would
rather raise an error instead of returning False. Conjugacy does not make
any sense for them. Moreover, everything can be tested at once using `if
parent(self) is not parent(other):`.
5. I learned that a lot of time is actually taken by the import
statements. You can try to minimize the number of them.
6. The usage of `quo_rem` looks weird. I thought that {{{q,r =
a.quo_rem(b)}}} would be equivalent to {{{q = a//b; r = a%b}}} but it is
not! For example {{{(1/3) % 5}}} returns `2` as it is the inverse of `3`
mod `5`. What do you think about the semantics of `//` and `%` against the
method `quo_rem`?
More to come...
Vincent
--
Ticket URL: <http://trac.sagemath.org/ticket/18505#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.
