fix: permute_axes bug (#1596)

As pointed out in #1589, permute_axes had a bug. This uses a simple
algorithm to fix it; explanations for this algorithm are all over the
internet, it's nothing fancy. I did take the opportunity to introduce
proptest, which I'd like to make more use of over time.

Down the line, it would be good to change the argument to `permute_axes`
to be something like `T: Permutation`. Right now, `permute_axes` just
has an assertion that the input is a real permutation. I'd call that a
classic example of this library opting for asserts when we could be
using better typing. But I think that will be easier / better to
introduce after the core rework.

Closes #1859

Author: akern40

.github/workflows/ci.yaml

@@ -44,7 +44,7 @@ jobs:
           toolchain: ${{ matrix.rust }}
           components: clippy
       - uses: Swatinem/rust-cache@v2
-      - run: cargo clippy --features approx,serde,rayon
+      - run: cargo clippy -F "${FEATURES}"
 
   format:
     runs-on: ubuntu-latest

Cargo.lock

@@ -57,6 +57,7 @@ quickcheck = { workspace = true }
 approx = { workspace = true, default-features = true }
 itertools = { workspace = true }
 ndarray-gen = { workspace = true }
+proptest = { workspace = true }
 
 [features]
 default = ["std"]
@@ -103,6 +104,7 @@ num-traits = { version = "0.2", default-features = false }
 num-complex = { version = "0.4", default-features = false }
 approx = { version = "0.5", default-features = false }
 quickcheck = { version = "1.0", default-features = false }
+proptest = { version = "1.3.1" }
 rand = { version = "0.9.0", features = ["small_rng"] }
 rand_distr = { version = "0.5.0" }
 itertools = { version = "0.13.0", default-features = false, features = ["use_std"] }

Cargo.toml

@@ -0,0 +1,7 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc 6df087160c6028416bca035db64caf430391dda0af3b195969776eb715184310 # shrinks to p = [0, 1, 2, 3, 4, 5]

proptest-regressions/impl_methods.txt

@@ -10,6 +10,14 @@ use alloc::slice;
 use alloc::vec;
 #[cfg(not(feature = "std"))]
 use alloc::vec::Vec;
+#[cfg(test)]
+use proptest::prop_assert_eq;
+#[cfg(test)]
+use proptest::proptest;
+#[cfg(test)]
+use proptest::strategy::Just;
+#[cfg(test)]
+use proptest::strategy::Strategy;
 #[allow(unused_imports)]
 use rawpointer::PointerExt;
 use std::mem::{size_of, ManuallyDrop};
@@ -2585,30 +2593,16 @@ where
 
         let dim = self.parts.dim.slice_mut();
         let strides = self.parts.strides.slice_mut();
-        let axes = axes.slice();
-
-        // The cycle detection is done using a bitmask to track visited positions.
-        // For example, axes from [0,1,2] to [2, 0, 1]
-        // For axis values [1, 0, 2]:
-        // 1 << 1  // 0b0001 << 1 = 0b0010 (decimal 2)
-        // 1 << 0  // 0b0001 << 0 = 0b0001 (decimal 1)
-        // 1 << 2  // 0b0001 << 2 = 0b0100 (decimal 4)
-        //
-        // Each axis gets its own unique bit position in the bitmask:
-        // - Axis 0: bit 0 (rightmost)
-        // - Axis 1: bit 1
-        // - Axis 2: bit 2
-        //
-        let mut visited = 0usize;
-        for (new_axis, &axis) in axes.iter().enumerate() {
-            if (visited & (1 << axis)) != 0 {
-                continue;
-            }
 
-            dim.swap(axis, new_axis);
-            strides.swap(axis, new_axis);
-
-            visited |= (1 << axis) | (1 << new_axis);
+        for i in 0..axes.ndim() {
+            let mut index = axes[i];
+            while index < i {
+                index = axes[index];
+            }
+            if index != i {
+                dim.swap(i, index);
+                strides.swap(i, index);
+            }
         }
     }
 
@@ -3614,4 +3608,24 @@ mod tests
         let result_slice = empty_slice.partition(0, Axis(0));
         assert_eq!(result_slice.shape(), &[0, 3]);
     }
+
+    /// Regression test for permute_axes
+    #[test]
+    fn test_permute_axes_regression()
+    {
+        let mut a = Array4::<u8>::zeros((1, 2, 3, 4));
+        a.permute_axes([3, 0, 1, 2]);
+        assert_eq!(a.shape(), &[4, 1, 2, 3]);
+    }
+}
+
+#[cfg(test)]
+#[cfg_attr(miri, ignore)]
+proptest! {
+    #[test]
+    fn test_permute_axes_6d(p in Just([0, 1, 2, 3, 4, 5]).prop_shuffle()) {
+        let mut arr: Array6<usize> = Array6::zeros((0, 1, 2, 3, 4, 5));
+        arr.permute_axes(p.clone());
+        prop_assert_eq!(arr.shape(), p);
+    }
 }