* Bug 14487 fixed: matrix indexing is now coherent with MATLAB 20/19920/11
St├ęphane Mottelet [Thu, 29 Mar 2018 10:36:31 +0000 (12:36 +0200)]
http://bugzilla.scilab.org/14487
http://bugzilla.scilab.org/11405

Change-Id: Ifd388cc9aa17075ca916c3bb35b468471eb80999

scilab/CHANGES.md
scilab/modules/ast/src/cpp/types/arrayof.cpp
scilab/modules/ast/tests/nonreg_tests/bug_14487.tst [new file with mode: 0644]
scilab/modules/core/tests/nonreg_tests/bug_11405.dia.ref [deleted file]
scilab/modules/core/tests/nonreg_tests/bug_11405.tst

index b9fe168..529b145 100644 (file)
@@ -306,6 +306,7 @@ Bug Fixes
 * [#14501](https://bugzilla.scilab.org/14501): `strsubst` crashed on consecutive occurrences.
 * [#14557](https://bugzilla.scilab.org/14557): `csim` failed when the system has no state.
 * [#14498](https://bugzilla.scilab.org/14498): `size([],3)` returned 1 instead of 0.
+* [#14487](https://bugzilla.scilab.org/14487): matrix indexing was not coherent with MATLAB. 
 * [#14502](https://bugzilla.scilab.org/14502): `Demo > GUI > Uicontrols 2` could not be exported to a file.
 * [#14604](https://bugzilla.scilab.org/14604): `emptystr` is 40x slower with 6.0.0 wrt 5.5.2
 * [#14605](https://bugzilla.scilab.org/14605): fixed - `bench_run` was too strict about the specification of tests names.
@@ -484,4 +485,3 @@ Bug Fixes
 * [#16321](https://bugzilla.scilab.org/16321): There were typo errors in the documentation.
 * [#16323](https://bugzilla.scilab.org/16323): `conj(sparse(x))` was complex when x is real.
 * [#16325](https://bugzilla.scilab.org/16325): `mgetl` could not read single line data which is greater than ~260,000 characters.
-
index aed56fe..d37f69f 100644 (file)
@@ -1326,7 +1326,7 @@ GenericType* ArrayOf<T>::extract(typed_list* _pArgs)
         }
     }
 
-    //vector
+    //linear indexing (one subscript)
     if (iDims == 1)
     {
         if (piCountDim[0] == 0)
@@ -1339,31 +1339,33 @@ GenericType* ArrayOf<T>::extract(typed_list* _pArgs)
         }
         else
         {
-            //two cases, depends of original matrix/vector
-            if ((*_pArgs)[0]->isColon() == false && m_iDims == 2 && m_piDims[1] != 1 && m_piDims[0] == 1)
+            int *i_piDims = pArg[0]->getAs<GenericType>()->getDimsArray();
+            if (!isScalar() && isVector() && (i_piDims[0] == 1 || i_piDims[1] == 1))
             {
-                //special case for row vector
-                int piRealDim[2] = {1, piCountDim[0]};
+                //vector with vector subscript
+                int piRealDim[2] = { 1, 1 };
+                piRealDim[(int)(m_piDims[0] == 1)] = piCountDim[0];
                 pOut = createEmpty(2, piRealDim, m_pImgData != NULL);
             }
             else
             {
-                if (getSize() == 1)
+                if ((*_pArgs)[0]->isBool())
                 {
-                    //for extraction on scalar
-                    pOut = createEmpty(pArg[0]->getAs<GenericType>()->getDims(), pArg[0]->getAs<GenericType>()->getDimsArray(), m_pImgData != NULL);
+                    //boolean extraction must return a column vector
+                    int piRealDim[2] = { piCountDim[0], 1 };
+                    pOut = createEmpty(2, piRealDim, m_pImgData != NULL);
                 }
                 else
                 {
-                    int piRealDim[2] = {piCountDim[0], 1};
-                    pOut = createEmpty(2, piRealDim, m_pImgData != NULL);
+                    //other cases
+                    pOut = createEmpty(pArg[0]->getAs<GenericType>()->getDims(), i_piDims, m_pImgData != NULL);
                 }
             }
         }
     }
     else
     {
-        //matrix
+        //indexing with more than one subscript
         pOut = createEmpty(iDims, piCountDim, m_pImgData != NULL);
     }
 
diff --git a/scilab/modules/ast/tests/nonreg_tests/bug_14487.tst b/scilab/modules/ast/tests/nonreg_tests/bug_14487.tst
new file mode 100644 (file)
index 0000000..71395bc
--- /dev/null
@@ -0,0 +1,62 @@
+// =============================================================================
+// Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
+// Copyright (C) 2018 - St├ęphane MOTTELET
+//
+//  This file is distributed under the same license as the Scilab package.
+// =============================================================================
+//
+// <-- CLI SHELL MODE -->
+// <-- NO CHECK REF -->
+//
+// <-- Non-regression test for bug 14487 -->
+//
+// <-- Bugzilla URL -->
+// http://bugzilla.scilab.org/14487
+//
+// <-- Short Description -->
+// Matrix indexing is not coherent with MATLAB convention
+
+x=rand(); // scalar
+i1=ones(1,4);
+i2=ones(2,4);
+i3=ones(2,2,2);
+assert_checkequal(size(x(i1)), [1 4]);
+assert_checkequal(size(x(i1')), [4 1]);
+assert_checkequal(size(x(i2)), [2 4]);
+assert_checkequal(size(x(i3)), [2 2 2]);
+
+x=rand(1,4); // row vector
+i1=ones(1,4);
+i2=ones(2,4);
+i3=ones(2,2,2);
+assert_checkequal(size(x(i1)), [1 4]);
+assert_checkequal(size(x(i1')), [1 4]);
+assert_checkequal(size(x(i2)), [2 4]);
+assert_checkequal(size(x(i3)), [2 2 2]);
+
+x=rand(4,1); // column vector
+i1=ones(1,4);
+i2=ones(2,4);
+i3=ones(2,2,2);
+assert_checkequal(size(x(i1)), [4 1]);
+assert_checkequal(size(x(i1')), [4 1]);
+assert_checkequal(size(x(i2)), [2 4]);
+assert_checkequal(size(x(i3)), [2 2 2]);
+
+x=rand(3,3); // matrix
+i1=ones(1,4);
+i2=ones(2,4);
+i3=ones(2,2,2);
+assert_checkequal(size(x(i1)), [1 4]);
+assert_checkequal(size(x(i1')), [4 1]);
+assert_checkequal(size(x(i2)), [2 4]);
+assert_checkequal(size(x(i3)), [2 2 2]);
+
+x=rand(3,3,3); // hypermatrix
+i1=ones(1,4);
+i2=ones(2,4);
+i3=ones(2,2,2);
+assert_checkequal(size(x(i1)), [1 4]);
+assert_checkequal(size(x(i1')), [4 1]);
+assert_checkequal(size(x(i2)), [2 4]);
+assert_checkequal(size(x(i3)), [2 2 2]);
diff --git a/scilab/modules/core/tests/nonreg_tests/bug_11405.dia.ref b/scilab/modules/core/tests/nonreg_tests/bug_11405.dia.ref
deleted file mode 100644 (file)
index 6b05550..0000000
+++ /dev/null
@@ -1,24 +0,0 @@
-// =============================================================================
-// Scilab ( http://www.scilab.org/ ) - This file is part of Scilab
-// Copyright (C) 2014 - Scilab Enterprises - Paul Bignier
-//
-//  This file is distributed under the same license as the Scilab package.
-// =============================================================================
-//
-// <-- Non-regression test for bug 11405 -->
-//
-// <-- CLI SHELL MODE -->
-//
-// <-- Bugzilla URL -->
-// http://bugzilla.scilab.org/11405
-//
-// <-- Short Description -->
-// Added hypermatrix support for extraction.
-vec = 0:4;
-indexes = matrix([1 1; 1 1; 3 3; 3 3], [2 2 2]);
-// Staying consistent with the way other extractions currently work,
-// so equivalent to vec(indexes(:)).
-assert_checkequal(vec(indexes), [0 0 2 2 0 0 2 2]);
-vec = [vec; vec];
-// Equivalent to vec(indexes(:)).
-assert_checkequal(vec(indexes), [0;0;1;1;0;0;1;1]);
index 5b65803..8305ba8 100644 (file)
@@ -8,6 +8,7 @@
 // <-- Non-regression test for bug 11405 -->
 //
 // <-- CLI SHELL MODE -->
+// <-- NO CHECK REF -->
 //
 // <-- Bugzilla URL -->
 // http://bugzilla.scilab.org/11405
 
 vec = 0:4;
 indexes = matrix([1 1; 1 1; 3 3; 3 3], [2 2 2]);
-// Staying consistent with the way other extractions currently work,
-// so equivalent to vec(indexes(:)).
-assert_checkequal(vec(indexes), [0 0 2 2 0 0 2 2]);
+// Staying consistent with the way other extractions work,
+// i.e. vec(indexes) has the size and dimensions of indexes
+assert_checkequal(vec(indexes), matrix([0 0 2 2 0 0 2 2],[2 2 2]));
 
 vec = [vec; vec];
-// Equivalent to vec(indexes(:)).
-assert_checkequal(vec(indexes), [0;0;1;1;0;0;1;1]);
+// vec(indexes) has the size and dimensions of indexes
+assert_checkequal(vec(indexes), matrix([0;0;1;1;0;0;1;1],[2 2 2]));