[ 
https://issues.apache.org/jira/browse/MATH-387?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12889530#action_12889530
 ] 

Luc Maisonobe commented on MATH-387:
------------------------------------

The optimizer that will construct the point does not know beforehand that the 
convergence checker selected by the user will use or not the point, so I don't 
think we can optimized away the cloning.

You can go ahead with allowing null everywhere, it is an improvement for the 
class that may be useful for some specific optimizers later on.

> Duplicate code
> --------------
>
>                 Key: MATH-387
>                 URL: https://issues.apache.org/jira/browse/MATH-387
>             Project: Commons Math
>          Issue Type: Improvement
>    Affects Versions: 2.1
>            Reporter: Gilles
>            Priority: Minor
>             Fix For: 2.2
>
>         Attachments: RealPointValuePair.java.diff
>
>
> In package optimization:
> {code:title=SimpleRealPointChecker.java|borderStyle=solid}
> public boolean converged(final int iteration, final RealPointValuePair 
> previous, final RealPointValuePair current) {
>     final double[] p        = previous.getPoint();
>     final double[] c        = current.getPoint();
>     for (int i = 0; i < p.length; ++i) {
>         final double difference = Math.abs(p[i] - c[i]);
>         final double size       = Math.max(Math.abs(p[i]), Math.abs(c[i]));
>         if ((difference > (size * relativeThreshold)) && (difference > 
> absoluteThreshold)) {
>             return false;
>         }
>     }
>     return true;
> }
> {code}
> {code:title=SimpleVectorialPointChecker.java|borderStyle=solid}
> public boolean converged(final int iteration, final VectorialPointValuePair 
> previous, final VectorialPointValuePair current) {
>     final double[] p = previous.getPointRef();
>     final double[] c = current.getPointRef();
>     for (int i = 0; i < p.length; ++i) {
>         final double pi         = p[i];
>         final double ci         = c[i];
>         final double difference = Math.abs(pi - ci);
>         final double size       = Math.max(Math.abs(pi), Math.abs(ci));
>         if ((difference > (size * relativeThreshold)) &&
>             (difference > absoluteThreshold)) {
>             return false;
>         }
>     }
>     return true;
> }
> {code}
> Do they do the same thing or am I missing something?
> Also in
> {code:title=SimpleScalarValueChecker.java|borderStyle=solid}
> public boolean converged(final int iteration, final RealPointValuePair 
> previous, final RealPointValuePair current) {
>     final double p          = previous.getValue();
>     final double c          = current.getValue();
>     final double difference = Math.abs(p - c);
>     final double size       = Math.max(Math.abs(p), Math.abs(c));
>     return (difference <= (size * relativeThreshold)) || (difference <= 
> absoluteThreshold);
> }
> {code}
> it seems overkill that one must create two {{RealPointValuePair}} objects 
> when one just wants to compare two {{double}}. Shouldn't this class contain a 
> method like
> {code}
> public boolean converged(int iteration, double previous, double current) {
>     final double difference = Math.abs(previous - current);
>     final double size       = Math.max(Math.abs(previous), Math.abs(current));
>     return (difference <= (size * relativeThreshold)) || (difference <= 
> absoluteThreshold);
> {code}
> ?
> Also none of these methods seem to need an {{iteration}} parameter.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to