[
https://issues.apache.org/jira/browse/MATH-581?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13080237#comment-13080237
]
Gilles commented on MATH-581:
-----------------------------
I just had a quick look; here are a few non-exhaustive remarks:
* You should add the license header at the top of all files.
* I'm not fond of keys like "theVector", "otherOperator". We could name them
according the parameter that triggered the exception e.g. in
{{AbstractPreconditionedIterativeLinearSolver}}: "operator_m" and "operator_a"
or just "m" and "a" (?).
* Couldn't the instance variables be made "final" (and assigned in the
constructors)?
* There is a class "Incrementor" in package "util" which should be used to
count things (e.g. iterations).
* If possible, I'd avoid cyclic references:
{code}
public void setMonitor(final AbstractIterativeLinearSolverMonitor monitor) {
this.monitor = monitor;
monitor.setSolver(this);
}
{code}
Also, you don't check that {{monitor}} is not null.
* Is a separate "monitor" class really necessary? Couldn't the monitoring be
part of {{AbstractIterativeLinearSolver}}, e.g. through a "shouldStop" abstract
method?
* {{StoppingCriterion2}} is an odd name if there isn't yet a
"StoppingCriterion" class. Are there other "monitor" types?
> Support for iterative linear solvers
> ------------------------------------
>
> Key: MATH-581
> URL: https://issues.apache.org/jira/browse/MATH-581
> Project: Commons Math
> Issue Type: New Feature
> Affects Versions: 3.0, Nightly Builds
> Reporter: Sébastien Brisard
> Labels: iterative, linear, solver
> Attachments: MATH-581-01.patch, MATH-581-02.zip, MATH-581-03.zip,
> MATH-581-04.zip, MATH-581-05.patch, MATH-581-05.patch,
> conjugate-gradient.zip, exceptions.patch, linearoperator.zip
>
>
> Dear all,
> this issue has already been discussed on the forum. The idea is to implement
> the most popular linear iterative solvers (CG, SYMMLQ, etc...) in
> commons-math. The beauty of these solvers is that they do not need direct
> access to the coefficients of the matrix, only matrix-vector products are
> necessary. This is goof, as sometimes it is inetficient to store the
> coefficients of the matrix.
> So basically, before implementing the iterative solvers, we need to define an
> interface slightly more general than a matrix, namely LinearOperator, with
> only one basic operation: matrix-vector product.
> Here are a few interfaces and abstract classes that do that. Nothing fancy
> yet, I just wanted to have you advice on the implementation before I commit
> some solvers.
> I thought these classes could go in a package
> org.apache.commons.math.linearoperator, but really, I haven't got a clue...
> Best regards,
> Sebastien
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira