On Fri, Oct 17, 2008 at 7:17 AM, Tim Kroeger <[EMAIL PROTECTED]> wrote: > Dear John/Roy/all > > On Thu, 16 Oct 2008, John Peterson wrote: > >> On Thu, Oct 16, 2008 at 10:46 AM, Roy Stogner <[EMAIL PROTECTED]> >> wrote: >>> >>> On Thu, 16 Oct 2008, Tim Kroeger wrote: >>> >>> I'm a bit unhappy with the current inheritance tree - there's no >>> inheritance between ShellMatrix and SparseMatrix, even though the >>> latter is already practically an abstract base class that declares the >>> functions the former will need to use? There's no getting around the >>> need to duplicate a lot of function definitions (get_diagonal() is >>> going to be different in the different classes no matter how things >>> are factored) but it seems like a little refactoring (make both >>> ShellMatrix and SparseMatrix inherit from a MatrixBase?) would at >>> least give us more concise code using them. > > That sounds right for me in a way, but on the other hand, I'm not quite > sure. I mean, it is difficult to decide which methods to include in that > class and which not. The first attempt would be to include exactly those > that are common to SparseMatrix and ShellMatrix (BTW, what about > DenseMatrix?), but perhaps somebody wants to implement another type of > matrix that has again more or less common properties with the matrices > implemented so far. If you don't mind, I would leave ShellMatrix as > completely disjoint from SparseMatrix for the moment. Of course, you may > feel free to combine them in a way once you included them into you > repository. > >> I think I'll withhold judgment until I've seen the new ShellMatrix >> header file...
After looking at the ShellMatrix instantiations from the patch, I'm now leaning towards Roy's suggestion that ShellMatrix should eventually be derived from SparseMatrix. I think adding an extra base class (eg. MatrixBase) for sparse matrices is unnecessary and maybe a bit confusing. Making all but one or two (how about m() and n()?) of the currently-pure-virtual functions in the SparseMatrix interface be calls to not_implemented() will help us a lot out in this endeavour. Anyway, my main reasons for thinking this way now are: 1.) The current ShellMatrix interface is only about five functions: m(), n(), vector_mult(), vector_mult_add(), and get_diagonal(). But if all goes well we may eventually want to, say, print a ShellMatrix or get its L1-norm or do any of the umpteen other capabilities currently supported by SparseMatrix. So this reason is really geared towards future-proofing the design. 2.) We would get the standard OOP capability that allows existing library routines taking a SparseMatrix reference or pointer to take a ShellMatrix pointer without other changes to the code! 3.) The SparseShellMatrix from Tim's patch is a great example of the "Proxy" Design Pattern (http://en.wikipedia.org/wiki/Proxy_pattern). In this pattern, both the proxy (SparseShellMatrix) and the the real subject (PetscMatrix) are generally derived from the same base (SparseMatrix) and the proxy's functionality is implemented in terms of the real subject. Deriving ShellMatrix (and therefore SparseShellMatrix) from SparseMatrix makes this relationship clear. I'm not stipulating this as a requirement for checking in the initial patch, of course. I volunteer to work on unifying the SparseMatrix/ShellMatrix hierarchy around once Tim's patch has been checked in. -- John ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ Libmesh-users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/libmesh-users
