#10347: Implementation of is_(skew_)symmetrizable for matrices
-----------------------------+----------------------------------------------
Reporter: stumpc5 | Owner: tbd
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-4.7.1
Component: combinatorics | Keywords: symmetrizable matrix
Work_issues: | Upstream: N/A
Reviewer: Hugh Thomas | Author: Christian Stump
Merged: | Dependencies:
-----------------------------+----------------------------------------------
Changes (by stumpc5):
* status: needs_work => needs_review
Comment:
Replying to [comment:11 hthomas]:
Hi Hugh -- thanks for your comments!
> * The description of the return value for is_skew_symmetrizable,
is_skew_symmetric, and _check_symmetrizability when return_diag is true
says that it returns the diagonal matrix, when in fact, it returns the
diagonal of the diagonal matrix. (I like the output as is, I just think
the documentation should change.)
done.
> * In _travel_column, technically, I think it makes more sense to check
that i isn't in dict before getting self_ik and self_ki, and testing
whether or not bool(self_ik) and bool(self_ki) agree -- if i is in dict,
this has already been checked.
done.
> * When return_diag is set, I don't see the point of saying [ d[i] for i
in sorted( d.keys() ) ]. Why not just say [ d[i] for i in range(
self._ncols ) ]?
done.
> * If the entries in the input matrix aren't in an integral domain,
things will get ugly. I realize that's far from the intended use case.
What's the protocol for something like that? Maybe just that the
docstring should say that the input is a matrix in an integral domain? Or
is it even necessary to do anything at all?
I added a sentence saying that the input matrix should be over an integral
domain (we actually need to be able to invert all nonzero entries we see
in the matrix - but the methods are not meant to be used like that and no
one is going to do so I guess). Also I added that the base ring must be
ordered if `positive=True`.
> * I don't really like the following, but, again, it's far from the
intended use case
There are many methods which do only make sense in certain contexts, I
don't really know how to avoid that...
> Or should I wait until the patchbot is also happy?
I hope it is now...
> Unless you think someone else should also review it, since I'm pretty
inexperienced.
If someone wanna have a look that's fine, but you can also give it a
positive review if no one is going to do so.
Thanks again! Christian
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10347#comment:13>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
