[ 
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


Reply via email to