#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
-~----------~----~----~----~------~----~------~--~---

Reply via email to