#6441: [with patch, needs review] Charpoly (plus adjoint and det)
----------------------------+-----------------------------------------------
Reporter: spancratz | Owner: somebody
Type: defect | Status: new
Priority: minor | Milestone: sage-4.1.1
Component: linear algebra | Keywords: charpoly, division-free
Reviewer: | Author: Sebastian Pancratz
Merged: |
----------------------------+-----------------------------------------------
Comment(by rbeezer):
Hi Sebastian,
1. I understand [Se02] a lot better now - well enough to have caught the
typo in Algorithm 3.1 (missing minus sign before last term in penultimate
line). That might be a good thing to mention in the docstring near the
citation.
2. So I should be OK with {{{_charpoly_df()}}} now. One question, why
did you choose to accumulate for sums using a construction like the
following?
{{{
for k in xrange(p):
F.set_unsafe(p, t, F.get_unsafe(p, t) - A.get_unsafe(k, t) *
F.get_unsafe(p-k-1, t))
}}}
A few lines above you used a temporary variable {{{s}}}, which is what I
would have expected later. Just wondering about the rationale?
3. I'm not sure {{{_is_certainly_field()}}} (and its integral domain
companion) are the best way around the "not-implemented" problem. I see
the problem, and understand the fix, but I'd rather see fewer global
functions in Sage and more object methods. And I am familiar with the
linear algebra routines, but less familiar with design decisions for
rings. Would you mind presenting the problem (and perhaps your solution)
on sage-devel and see if there is a cleaner way to solve it or some kind
of explanation? Or perhaps your solution is the best idea. Me, I'd
probably just try to handle the exception where it happens rather than
make a function that looks so much like {{{is_field()}}}, but isn't really
the same at all.
4. You've included lots of formatting fixes - which is fantastic. But
Sage likes to have small patches that address one thing. How hard would
it be for you to separate the documentation clean-up from the division-
free stuff? For example, right now, it is hard to notice where your
changes to the determinant have affected doctests in unrelated modules,
since they are mixed in with lots of other small changes. You could put
the formatting fixes in a new ticket (e.g. "docstring clean-up for
matrices"), and I could review that patch very quickly. Then when
somebody comes back to look at the df stuff, they can see exactly what
changed, while nobody will probably ever look at the dosctring clean-up
patch again. and you'll get credit for two patches. ;-)
As I've said, this is a great addition to Sage and very carefully done,
which is really appreciated. None of the above will keep me from looking
at this some more in the next couple of days, so at your leisure.
Rob
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/6441#comment:9>
Sage <http://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
-~----------~----~----~----~------~----~------~--~---