Commons Math
  1. Commons Math
  2. MATH-803

Bugs in RealVector.ebeMultiply(RealVector) and ebeDivide(RealVector)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.0
    • Fix Version/s: 3.3
    • Labels:
      None

      Description

      OpenMapRealVector.ebeMultiply(RealVector) and OpenMapRealVector.ebeDivide(RealVector) return wrong values when one entry of the specified RealVector is nan or infinity. The bug is easy to understand. Here is the current implementation of ebeMultiply

          public OpenMapRealVector ebeMultiply(RealVector v) {
              checkVectorDimensions(v.getDimension());
              OpenMapRealVector res = new OpenMapRealVector(this);
              Iterator iter = entries.iterator();
              while (iter.hasNext()) {
                  iter.advance();
                  res.setEntry(iter.key(), iter.value() * v.getEntry(iter.key()));
              }
              return res;
          }
      

      The assumption is that for any double x, x * 0d == 0d holds, which is not true. The bug is easy enough to identify, but more complex to solve. The only solution I can come up with is to loop through all entries of v (instead of those entries which correspond to non-zero entries of this). I'm afraid about performance losses.

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        620d 21h 7m 1 Luc Maisonobe 20/Feb/14 15:58
        Resolved Resolved Closed Closed
        87d 23h 14m 1 Luc Maisonobe 19/May/14 16:13
        Luc Maisonobe made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Luc Maisonobe added a comment -

        Closing all resolved issue now available in released 3.3 version.

        Show
        Luc Maisonobe added a comment - Closing all resolved issue now available in released 3.3 version.
        Luc Maisonobe made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 3.3 [ 12324600 ]
        Fix Version/s 4.0 [ 12317577 ]
        Resolution Fixed [ 1 ]
        Hide
        Luc Maisonobe added a comment -

        Methods undeprecated in r1570246.
        The test cases have been adapted, and the fact we force 0 * x = 0 even for NaNs and infinities is considered acceptable now, as it is similar to standard practive for sparse linear algebra libraries.

        Show
        Luc Maisonobe added a comment - Methods undeprecated in r1570246. The test cases have been adapted, and the fact we force 0 * x = 0 even for NaNs and infinities is considered acceptable now, as it is similar to standard practive for sparse linear algebra libraries.
        Gilles made changes -
        Fix Version/s 4.0 [ 12317577 ]
        Gilles made changes -
        Affects Version/s 3.0 [ 12314824 ]
        Affects Version/s 4.0 [ 12317577 ]
        Sébastien Brisard made changes -
        Assignee Sébastien Brisard [ celestin ]
        Sébastien Brisard made changes -
        Affects Version/s 4.0 [ 12317577 ]
        Affects Version/s 3.0 [ 12314824 ]
        Hide
        Sébastien Brisard added a comment -

        Methods are deprecated in r1352782, and unit tests are skipped (they are kept for reference). Issue is to remain open until the methods are removed in version 4.0.

        Show
        Sébastien Brisard added a comment - Methods are deprecated in r1352782 , and unit tests are skipped (they are kept for reference). Issue is to remain open until the methods are removed in version 4.0.
        Hide
        Sébastien Brisard added a comment -

        According to this thread, ebeMultiply and ebeDivide are deprecated.

        Show
        Sébastien Brisard added a comment - According to this thread , ebeMultiply and ebeDivide are deprecated.
        Sébastien Brisard made changes -
        Field Original Value New Value
        Summary Bugs in OpenMapRealVector.ebeMultiply(RealVector) and ebeDivide(RealVector) Bugs in RealVector.ebeMultiply(RealVector) and ebeDivide(RealVector)
        Hide
        Sébastien Brisard added a comment -

        Changed the title of the ticket, since the scope of this bug is much broader. Indeed, it affects RealVector.ebeMultiply(RealVector) and RealVector.ebeDivide(RealVector) as soon as the RealVector passed as a parameter is sparse, see examples below

        1. for ebeMultiply()
          final RealVector v1 = new ArrayRealVector(new double[] { 1d });
          final RealVector v2 = new OpenMapRealVector(new double[] { -0d });
          final RealVector w = v1.ebeMultiply(v2);
          System.out.println(1d / w.getEntry(0));
          

          prints Infinity, instead of -Infinity (because the sign is lost in v2). This means that w holds +0d instead of -0d.

        2. for ebeDivide()
          final RealVector v1 = new ArrayRealVector(new double[] { 1d });
          final RealVector v2 = new OpenMapRealVector(new double[] { -0d });
          final RealVector w = v1.ebeDivide(v2);
          System.out.println(w.getEntry(0));
          

          prints Infinity, instead of -Infinity.

        Show
        Sébastien Brisard added a comment - Changed the title of the ticket, since the scope of this bug is much broader. Indeed, it affects RealVector.ebeMultiply(RealVector) and RealVector.ebeDivide(RealVector) as soon as the RealVector passed as a parameter is sparse, see examples below for ebeMultiply() final RealVector v1 = new ArrayRealVector( new double [] { 1d }); final RealVector v2 = new OpenMapRealVector( new double [] { -0d }); final RealVector w = v1.ebeMultiply(v2); System .out.println(1d / w.getEntry(0)); prints Infinity , instead of -Infinity (because the sign is lost in v2 ). This means that w holds +0d instead of -0d . for ebeDivide() final RealVector v1 = new ArrayRealVector( new double [] { 1d }); final RealVector v2 = new OpenMapRealVector( new double [] { -0d }); final RealVector w = v1.ebeDivide(v2); System .out.println(w.getEntry(0)); prints Infinity , instead of -Infinity .
        Hide
        Sébastien Brisard added a comment -

        I think this does not work for ebeDivide(RealVector). In this case, I suggest to revert to naive implementation (loop through all entries).

        Show
        Sébastien Brisard added a comment - I think this does not work for ebeDivide(RealVector) . In this case, I suggest to revert to naive implementation (loop through all entries).
        Hide
        Sébastien Brisard added a comment -

        I'm going to implement the suggested post-processing for the time being; this should solve the bug. Caching of isNaN() and isInfinite() is postponed.

        Show
        Sébastien Brisard added a comment - I'm going to implement the suggested post-processing for the time being; this should solve the bug. Caching of isNaN() and isInfinite() is postponed.
        Hide
        Luc Maisonobe added a comment -

        Perhaps we should use a post-processing branch to handle infinite or spacial values:

        // current implementation looping over non-zero instance elements goes here
        
        if (v.isNaN() || v.isInfinite()) {
          // post-processing loop, to handle 0 * infinity and 0 * NaN cases
          for (int i = 0; i < v.getDimension(); ++v) {
            if (Double.isInfinite(v.getElement(i)) || Double.isNaN(v.getElement(i)) {
               res.setEntry(i, Double.NaN);
            }
          }
        }
        

        This could be fast only if isNaN() and isInfinite() results are cached and the same vector is reused, otherwise the outer if statement should be removed and the post-processing should be done in all cases.

        Show
        Luc Maisonobe added a comment - Perhaps we should use a post-processing branch to handle infinite or spacial values: // current implementation looping over non-zero instance elements goes here if (v.isNaN() || v.isInfinite()) { // post-processing loop, to handle 0 * infinity and 0 * NaN cases for ( int i = 0; i < v.getDimension(); ++v) { if ( Double .isInfinite(v.getElement(i)) || Double .isNaN(v.getElement(i)) { res.setEntry(i, Double .NaN); } } } This could be fast only if isNaN() and isInfinite() results are cached and the same vector is reused, otherwise the outer if statement should be removed and the post-processing should be done in all cases.
        Hide
        Sébastien Brisard added a comment -

        In r1348485, RealVectorAbstractTest include unit tests illustrating this bug.

        Show
        Sébastien Brisard added a comment - In r1348485 , RealVectorAbstractTest include unit tests illustrating this bug.
        Sébastien Brisard created issue -

          People

          • Assignee:
            Unassigned
            Reporter:
            Sébastien Brisard
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development