----- "luc maisonobe" <[EMAIL PROTECTED]> a écrit :
> ----- "Phil Steitz" <[EMAIL PROTECTED]> a écrit :
>
>
> > Looks good. Just one last question / suggestion. In the
> > DecompositionSolver, why to we keep the argumentless decompose()
> > methods
> > and the matrix as an instance variable? The argumentless methods
> just
> >
> > delegate to the ones that take a matrix as an argument and the
> > instance
> > variable is otherwise unused.
> >
> > Phil
>
> This also bothers me. I tried different things and ended up with that
> but didn't feel comfortable with it. Since it was already very late, I
> commited it for review.
>
> In addition to the fact the instance field is almost not used, the
> calling code is awkward:
>
> DecompositionSolver solver = new DecompositionSolver(matrix);
> QRDecomposition decomposition = solver.qrDecompose();
> RealVector solution = solver.solve(constant, decomposition);
>
> I would prefer a one-liner. After one night sleeping, I see two better
> approaches. The first one would be to remove completely the field
> instance, the xyzDecompose methods and change all other methods
> (solve, isNonSingular ...) to static, thus making DecompositionSolver
> a utility class never instantiated. The second one would be to have a
> DecompositionSolver that can be instantiated but instead of storing
> the matrix, it would store a configuration flag specifying which
> decomposition algorithm to use, in this case, the undecomposed matrix
> would be passed in the solve method and decomposed on the fly.
>
> The first approach leads to:
> RealVector solution =
> DecompositionSolver.solve(constant, new
> QRDecompositionImpl(matrix));
>
> The second approach leads to:
> RealVector solution =
> new
> DecompositionSolver(DecompositionSolver.USE_QR).solve(constant,
> matrix);
>
> I prefer the first one. The decomposition algorithm is more explicit
> to users and they can reuse it. The second approach only advantage is
> if someone wants to configure a solver with some algorithm type once
> and for all and to reuse it several time in his code. However I think
> this use case is not a common one and it could be done by a suitable
> wrapper.
>
> There is also one other thing I don't really like in my current
> implementation: the overloaded methods with the four types of
> decomposition (EigenDecomposition, SingularValuesDecomposition,
> QRDecomposition, LUDecomposition). However, simplifying this would
> require reintroducing some common base interface and wrappers like
> this:
>
> public class Solver {
> public RealVector solve(RealVector constant) {
> ...
> }
> }
> public class QRSolver implements Solver {
> ...
> }
>
> public class DecompositionSolver {
> public RealVector solve(RealVector constant, Solver solver) {
> return solver.solve(constant);
> }
> }
>
> and we would end up with a DecompositionSolver that has no purpose at
> all! So I think it is better to stick with the four signatures types.
I'm stupid!
Since DecompositionSolver does not have any purpose anymore, just remove it,
and reuse
its name for the Solver interface. So we would have:
public interface DecompositionSolver extends Serializable {
RealVector solve(RealVector b);
realMatrix getInverse();
...
}
public class QRSolver implements DecompositionSolver {
public QRSolver(QRDecomposition decomposition) {
this.decomposition = decomposition;
}
public RealVector solve(RealVector b) {
RealMatrix q = decomposition.getQ();
...
}
}
and user code would look like:
RealVector solution = new QRSolver(new QRDecomposition()).solve(constant);
>From users point of view this is a one-liner, simple to understand, all
>elements can be reused if needed (the solver, the decomposition by itself).
>From a design point of view, this separates cleanly the decomposition from the
>solve, it is extensible if we want to add new decomposition algorithms and the
>interface is not cluttered.
I'll try this.
Sorry for the noise.
Luc
>
> I will therefore implement and commit today the first approach without
> the four signatures types, just to see how it looks once implemented.
> It is straightforward and could be changed afterwards if something
> better is found.
>
>
> Luc
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
