[jira] Created: (MATH-326) getLInfNorm() uses wrong formula in both ArrayRealVector and OpenMapRealVector (in different ways)

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[jira] Created: (MATH-326) getLInfNorm() uses wrong formula in both ArrayRealVector and OpenMapRealVector (in different ways)

ASF GitHub Bot (Jira)
getLInfNorm() uses wrong formula in both ArrayRealVector and OpenMapRealVector (in different ways)
--------------------------------------------------------------------------------------------------

                 Key: MATH-326
                 URL: https://issues.apache.org/jira/browse/MATH-326
             Project: Commons Math
          Issue Type: Bug
    Affects Versions: 2.0
         Environment: all
            Reporter: Jake Mannix


the L_infinity norm of a finite dimensional vector is just the max of the absolute value of its entries.

The current implementation in ArrayRealVector has a typo:

{code}
    public double getLInfNorm() {
        double max = 0;
        for (double a : data) {
            max += Math.max(max, Math.abs(a));
        }
        return max;
    }
{code}

the += should just be an =.

There is sadly a unit test assuring us that this is the correct behavior (effectively a regression-only test, not a test for correctness).

Worse, the implementation in OpenMapRealVector is not even positive semi-definite:

{code}  
    public double getLInfNorm() {
        double max = 0;
        Iterator iter = entries.iterator();
        while (iter.hasNext()) {
            iter.advance();
            max += iter.value();
        }
        return max;
    }
{code}

I would suggest that this method be moved up to the AbstractRealVector superclass and implemented using the sparseIterator():

{code}
  public double getLInfNorm() {
    double norm = 0;
    Iterator<Entry> it = sparseIterator();
    Entry e;
    while(it.hasNext() && (e = it.next()) != null) {
      norm = Math.max(norm, Math.abs(e.getValue()));
    }
    return norm;
  }
{code}

Unit tests with negative valued vectors would be helpful to check for this kind of thing in the future.

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

Reply | Threaded
Open this post in threaded view
|

[jira] Resolved: (MATH-326) getLInfNorm() uses wrong formula in both ArrayRealVector and OpenMapRealVector (in different ways)

ASF GitHub Bot (Jira)

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

Luc Maisonobe resolved MATH-326.
--------------------------------

    Resolution: Fixed

Fixed in subversion repository as of r894367.
For consistency, the implementation of L1 and L2 norms have also been pushed upward in the abstract class.
Thanks for reporting and providing a fix to this bug

> getLInfNorm() uses wrong formula in both ArrayRealVector and OpenMapRealVector (in different ways)
> --------------------------------------------------------------------------------------------------
>
>                 Key: MATH-326
>                 URL: https://issues.apache.org/jira/browse/MATH-326
>             Project: Commons Math
>          Issue Type: Bug
>    Affects Versions: 2.0
>         Environment: all
>            Reporter: Jake Mannix
>
> the L_infinity norm of a finite dimensional vector is just the max of the absolute value of its entries.
> The current implementation in ArrayRealVector has a typo:
> {code}
>     public double getLInfNorm() {
>         double max = 0;
>         for (double a : data) {
>             max += Math.max(max, Math.abs(a));
>         }
>         return max;
>     }
> {code}
> the += should just be an =.
> There is sadly a unit test assuring us that this is the correct behavior (effectively a regression-only test, not a test for correctness).
> Worse, the implementation in OpenMapRealVector is not even positive semi-definite:
> {code}  
>     public double getLInfNorm() {
>         double max = 0;
>         Iterator iter = entries.iterator();
>         while (iter.hasNext()) {
>             iter.advance();
>             max += iter.value();
>         }
>         return max;
>     }
> {code}
> I would suggest that this method be moved up to the AbstractRealVector superclass and implemented using the sparseIterator():
> {code}
>   public double getLInfNorm() {
>     double norm = 0;
>     Iterator<Entry> it = sparseIterator();
>     Entry e;
>     while(it.hasNext() && (e = it.next()) != null) {
>       norm = Math.max(norm, Math.abs(e.getValue()));
>     }
>     return norm;
>   }
> {code}
> Unit tests with negative valued vectors would be helpful to check for this kind of thing in the future.

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

Reply | Threaded
Open this post in threaded view
|

[jira] Closed: (MATH-326) getLInfNorm() uses wrong formula in both ArrayRealVector and OpenMapRealVector (in different ways)

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

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

Phil Steitz closed MATH-326.
----------------------------


> getLInfNorm() uses wrong formula in both ArrayRealVector and OpenMapRealVector (in different ways)
> --------------------------------------------------------------------------------------------------
>
>                 Key: MATH-326
>                 URL: https://issues.apache.org/jira/browse/MATH-326
>             Project: Commons Math
>          Issue Type: Bug
>    Affects Versions: 2.0
>         Environment: all
>            Reporter: Jake Mannix
>             Fix For: 2.1
>
>
> the L_infinity norm of a finite dimensional vector is just the max of the absolute value of its entries.
> The current implementation in ArrayRealVector has a typo:
> {code}
>     public double getLInfNorm() {
>         double max = 0;
>         for (double a : data) {
>             max += Math.max(max, Math.abs(a));
>         }
>         return max;
>     }
> {code}
> the += should just be an =.
> There is sadly a unit test assuring us that this is the correct behavior (effectively a regression-only test, not a test for correctness).
> Worse, the implementation in OpenMapRealVector is not even positive semi-definite:
> {code}  
>     public double getLInfNorm() {
>         double max = 0;
>         Iterator iter = entries.iterator();
>         while (iter.hasNext()) {
>             iter.advance();
>             max += iter.value();
>         }
>         return max;
>     }
> {code}
> I would suggest that this method be moved up to the AbstractRealVector superclass and implemented using the sparseIterator():
> {code}
>   public double getLInfNorm() {
>     double norm = 0;
>     Iterator<Entry> it = sparseIterator();
>     Entry e;
>     while(it.hasNext() && (e = it.next()) != null) {
>       norm = Math.max(norm, Math.abs(e.getValue()));
>     }
>     return norm;
>   }
> {code}
> Unit tests with negative valued vectors would be helpful to check for this kind of thing in the future.

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

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (MATH-326) getLInfNorm() uses wrong formula in both ArrayRealVector and OpenMapRealVector (in different ways)

ASF GitHub Bot (Jira)
In reply to this post by ASF GitHub Bot (Jira)

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

Phil Steitz updated MATH-326:
-----------------------------

    Fix Version/s: 2.1

> getLInfNorm() uses wrong formula in both ArrayRealVector and OpenMapRealVector (in different ways)
> --------------------------------------------------------------------------------------------------
>
>                 Key: MATH-326
>                 URL: https://issues.apache.org/jira/browse/MATH-326
>             Project: Commons Math
>          Issue Type: Bug
>    Affects Versions: 2.0
>         Environment: all
>            Reporter: Jake Mannix
>             Fix For: 2.1
>
>
> the L_infinity norm of a finite dimensional vector is just the max of the absolute value of its entries.
> The current implementation in ArrayRealVector has a typo:
> {code}
>     public double getLInfNorm() {
>         double max = 0;
>         for (double a : data) {
>             max += Math.max(max, Math.abs(a));
>         }
>         return max;
>     }
> {code}
> the += should just be an =.
> There is sadly a unit test assuring us that this is the correct behavior (effectively a regression-only test, not a test for correctness).
> Worse, the implementation in OpenMapRealVector is not even positive semi-definite:
> {code}  
>     public double getLInfNorm() {
>         double max = 0;
>         Iterator iter = entries.iterator();
>         while (iter.hasNext()) {
>             iter.advance();
>             max += iter.value();
>         }
>         return max;
>     }
> {code}
> I would suggest that this method be moved up to the AbstractRealVector superclass and implemented using the sparseIterator():
> {code}
>   public double getLInfNorm() {
>     double norm = 0;
>     Iterator<Entry> it = sparseIterator();
>     Entry e;
>     while(it.hasNext() && (e = it.next()) != null) {
>       norm = Math.max(norm, Math.abs(e.getValue()));
>     }
>     return norm;
>   }
> {code}
> Unit tests with negative valued vectors would be helpful to check for this kind of thing in the future.

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