[jira] Updated: (MATH-387) Duplicate code

["Gilles (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/MATH-387?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gilles updated MATH-387:
------------------------

    Attachment: RealPointValuePair.java.diff

Forget about the initial description; fixing it will probably entail incompatible changes.

Nevertheless I'd like to modify "RealPointValuePair.java" (see attached diff) so that it can be made fairly obvious (in an algorithm's code) that the convergence checker only uses the value of the objective function.

Objections?


> 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.