Skip to content

Fix bug in layout and broken debug assertion in collect to f-order array #900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions src/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,30 @@ mod tests {
use crate::NdProducer;

type M = Array2<f32>;
type M1 = Array1<f32>;
type M0 = Array0<f32>;

macro_rules! assert_layouts {
($mat:expr, $($layout:expr),*) => {{
let layout = $mat.view().layout();
$(
assert!(layout.is($layout), "Assertion failed: array {:?} is not layout {}",
$mat,
stringify!($layout));
)*
}}
}

macro_rules! assert_not_layouts {
($mat:expr, $($layout:expr),*) => {{
let layout = $mat.view().layout();
$(
assert!(!layout.is($layout), "Assertion failed: array {:?} show not have layout {}",
$mat,
stringify!($layout));
)*
}}
}

#[test]
fn contig_layouts() {
Expand All @@ -91,6 +115,34 @@ mod tests {
assert!(af.is(FORDER) && af.is(FPREFER));
}

#[test]
fn contig_cf_layouts() {
let a = M::zeros((5, 1));
let b = M::zeros((1, 5).f());
assert_layouts!(a, CORDER, CPREFER, FORDER, FPREFER);
assert_layouts!(b, CORDER, CPREFER, FORDER, FPREFER);

let a = M1::zeros(5);
let b = M1::zeros(5.f());
assert_layouts!(a, CORDER, CPREFER, FORDER, FPREFER);
assert_layouts!(b, CORDER, CPREFER, FORDER, FPREFER);

let a = M0::zeros(());
assert_layouts!(a, CORDER, CPREFER, FORDER, FPREFER);

let a = M::zeros((5, 5));
let b = M::zeros((5, 5).f());
let arow = a.slice(s![..1, ..]);
let bcol = b.slice(s![.., ..1]);
assert_layouts!(arow, CORDER, CPREFER, FORDER, FPREFER);
assert_layouts!(bcol, CORDER, CPREFER, FORDER, FPREFER);

let acol = a.slice(s![.., ..1]);
let brow = b.slice(s![..1, ..]);
assert_not_layouts!(acol, CORDER, CPREFER, FORDER, FPREFER);
assert_not_layouts!(brow, CORDER, CPREFER, FORDER, FPREFER);
}

#[test]
fn stride_layouts() {
let a = M::zeros((5, 5));
Expand Down
18 changes: 15 additions & 3 deletions src/zip/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ where
pub(crate) fn layout_impl(&self) -> Layout {
let n = self.ndim();
if self.is_standard_layout() {
if n <= 1 {
// effectively one-dimensional => C and F layout compatible
if n <= 1 || self.shape().iter().filter(|&&len| len > 1).count() <= 1 {
Layout::one_dimensional()
} else {
Layout::c()
Expand Down Expand Up @@ -1173,8 +1174,19 @@ macro_rules! map_impl {
{
// Get the last producer; and make a Partial that aliases its data pointer
let (.., ref output) = &self.parts;
debug_assert!(output.layout().is(CORDER | FORDER));
debug_assert_eq!(output.layout().tendency() >= 0, self.layout_tendency >= 0);

// debug assert that the output is contiguous in the memory layout we need
if cfg!(debug_assertions) {
let out_layout = output.layout();
assert!(out_layout.is(CORDER | FORDER));
assert!(
(self.layout_tendency <= 0 && out_layout.tendency() <= 0) ||
(self.layout_tendency >= 0 && out_layout.tendency() >= 0),
"layout tendency violation for self layout {:?}, output layout {:?},\
output shape {:?}",
self.layout, out_layout, output.raw_dim());
}

let mut partial = Partial::new(output.as_ptr());

// Apply the mapping function on this zip
Expand Down
24 changes: 15 additions & 9 deletions tests/par_zip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,20 +107,26 @@ fn test_zip_small_collect() {

for m in 0..32 {
for n in 0..16 {
let dim = (m, n);
let b = Array::from_shape_fn(dim, |(i, j)| 1. / (i + 2 * j + 1) as f32);
let c = Array::from_shape_fn(dim, |(i, j)| f32::ln((1 + i + j) as f32));

{
let a = Zip::from(&b).and(&c).par_map_collect(|x, y| x + y);

assert_abs_diff_eq!(a, &b + &c, epsilon = 1e-6);
assert_eq!(a.strides(), b.strides());
for &is_f in &[false, true] {
let dim = (m, n).set_f(is_f);
let b = Array::from_shape_fn(dim, |(i, j)| 1. / (i + 2 * j + 1) as f32);
let c = Array::from_shape_fn(dim, |(i, j)| f32::ln((1 + i + j) as f32));

{
let a = Zip::from(&b).and(&c).par_map_collect(|x, y| x + y);

assert_abs_diff_eq!(a, &b + &c, epsilon = 1e-6);
if m > 1 && n > 1 {
assert_eq!(a.strides(), b.strides(),
"Failure for {}x{} c/f: {:?}", m, n, is_f);
}
}
}
}
}
}


#[test]
#[cfg(feature = "approx")]
fn test_zip_assign_into() {
Expand Down