Skip to content
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

Encode expected/actual info in ShapeError #962

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
FIX: Add extra information to errors where possible
Where possible, add expected/actual information to the ShapeError.

In many places it is identified new places where more specific
ErrorKinds and error messages are needed. These are not updated here - a
comment is inserted - this will be updated in a future version, when we
can accept breaking changes.
  • Loading branch information
bluss committed Mar 29, 2021
commit 5f34c4ac868a3089ccabe58a2e353831e289d72a
1 change: 1 addition & 0 deletions src/dimension/broadcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ where
if *out == 1 {
*out = *s2
} else if *s2 != 1 {
// TODO More specific error axis length mismatch
return Err(from_kind(ErrorKind::IncompatibleShape));
}
}
Expand Down
16 changes: 12 additions & 4 deletions src/dimension/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ pub fn size_of_shape_checked<D: Dimension>(dim: &D) -> Result<usize, ShapeError>
.try_fold(1usize, |acc, &d| acc.checked_mul(d))
.ok_or_else(|| from_kind(ErrorKind::Overflow))?;
if size_nonzero > ::std::isize::MAX as usize {
// TODO More specific error
Err(from_kind(ErrorKind::Overflow))
} else {
Ok(dim.size())
Expand Down Expand Up @@ -137,7 +138,7 @@ pub(crate) fn can_index_slice_not_custom<D: Dimension>(data_len: usize, dim: &D)
let len = size_of_shape_checked(dim)?;
// Condition 2.
if len > data_len {
return Err(from_kind(ErrorKind::OutOfBounds));
return Err(ShapeError::shape_length_exceeds_data_length(data_len, len));
}
Ok(())
}
Expand Down Expand Up @@ -170,6 +171,7 @@ where
{
// Condition 1.
if dim.ndim() != strides.ndim() {
// TODO More specific error for dimension stride dimensionality mismatch
return Err(from_kind(ErrorKind::IncompatibleLayout));
}

Expand All @@ -185,19 +187,23 @@ where
let off = d.saturating_sub(1).checked_mul(s.abs() as usize)?;
acc.checked_add(off)
})
// TODO More specific error
.ok_or_else(|| from_kind(ErrorKind::Overflow))?;
// Condition 2a.
if max_offset > isize::MAX as usize {
// TODO More specific error
return Err(from_kind(ErrorKind::Overflow));
}

// Determine absolute difference in units of bytes between least and
// greatest address accessible by moving along all axes
let max_offset_bytes = max_offset
.checked_mul(elem_size)
// TODO More specific error
.ok_or_else(|| from_kind(ErrorKind::Overflow))?;
// Condition 2b.
if max_offset_bytes > isize::MAX as usize {
// TODO More specific error
return Err(from_kind(ErrorKind::Overflow));
}

Expand Down Expand Up @@ -256,15 +262,16 @@ fn can_index_slice_impl<D: Dimension>(
// Check condition 3.
let is_empty = dim.slice().iter().any(|&d| d == 0);
if is_empty && max_offset > data_len {
return Err(from_kind(ErrorKind::OutOfBounds));
return Err(ShapeError::shape_length_exceeds_data_length(data_len, max_offset));
}
if !is_empty && max_offset >= data_len {
return Err(from_kind(ErrorKind::OutOfBounds));
return Err(ShapeError::shape_length_exceeds_data_length(data_len.wrapping_sub(1), max_offset));
}

// Check condition 4.
if !is_empty && dim_stride_overlap(dim, strides) {
return Err(from_kind(ErrorKind::Unsupported));
// TODO: More specific error kind Strides result in overlapping elements
return Err(ShapeError::from_kind(ErrorKind::Unsupported));
}

Ok(())
Expand Down Expand Up @@ -293,6 +300,7 @@ where
{
for &stride in strides.slice() {
if (stride as isize) < 0 {
// TODO: More specific error kind Non-negative strides required
return Err(from_kind(ErrorKind::Unsupported));
}
}
Expand Down
14 changes: 9 additions & 5 deletions src/impl_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::dimension::{
offset_from_ptr_to_memory, size_of_shape_checked, stride_offset, Axes,
};
use crate::dimension::broadcast::co_broadcast;
use crate::error::{self, ErrorKind, ShapeError, from_kind};
use crate::error::{self, ErrorKind, ShapeError};
use crate::math_cell::MathCell;
use crate::itertools::zip;
use crate::zip::{IntoNdProducer, Zip};
Expand Down Expand Up @@ -1588,7 +1588,7 @@ where
} else if self.ndim() > 1 && self.raw_view().reversed_axes().is_standard_layout() {
Ok(self.with_strides_dim(shape.fortran_strides(), shape))
} else {
Err(error::from_kind(error::ErrorKind::IncompatibleLayout))
Err(ShapeError::incompatible_layout(error::ExpectedLayout::ContiguousCF))
}
}
}
Expand Down Expand Up @@ -1693,6 +1693,7 @@ where
}
}
}
// TODO More specific error incompatible ndim
Err(ShapeError::from_kind(ErrorKind::IncompatibleShape))
}

Expand Down Expand Up @@ -1805,11 +1806,14 @@ where
{
let shape = co_broadcast::<D, E, <D as DimMax<E>>::Output>(&self.dim, &other.dim)?;
if let Some(view1) = self.broadcast(shape.clone()) {
if let Some(view2) = other.broadcast(shape) {
return Ok((view1, view2));
if let Some(view2) = other.broadcast(shape.clone()) {
Ok((view1, view2))
} else {
Err(ShapeError::incompatible_shapes(&other.dim, &shape))
}
} else {
Err(ShapeError::incompatible_shapes(&other.dim, &shape))
}
Err(from_kind(ErrorKind::IncompatibleShape))
}

/// Swap axes `ax` and `bx`.
Expand Down
2 changes: 2 additions & 0 deletions src/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,11 +415,13 @@ where
{
if let Some(in_ndim) = Din::NDIM {
if in_ndim != indices.in_ndim() {
// TODO More specific error incompatible ndim
return Err(ShapeError::from_kind(ErrorKind::IncompatibleShape));
}
}
if let Some(out_ndim) = Dout::NDIM {
if out_ndim != indices.out_ndim() {
// TODO More specific error incompatible ndim
return Err(ShapeError::from_kind(ErrorKind::IncompatibleShape));
}
}
Expand Down
24 changes: 16 additions & 8 deletions src/stacking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

use crate::error::{from_kind, ErrorKind, ShapeError};
use crate::imp_prelude::*;
use crate::NdProducer;

/// Stack arrays along the new axis.
///
Expand Down Expand Up @@ -72,18 +73,22 @@ where
D: RemoveAxis,
{
if arrays.is_empty() {
// TODO More specific error for empty input not supported
return Err(from_kind(ErrorKind::Unsupported));
}
let mut res_dim = arrays[0].raw_dim();
if axis.index() >= res_dim.ndim() {
return Err(from_kind(ErrorKind::OutOfBounds));
return Err(ShapeError::invalid_axis(res_dim.ndim().wrapping_sub(1), axis.index()));
}
let common_dim = res_dim.remove_axis(axis);
if arrays
.iter()
.any(|a| a.raw_dim().remove_axis(axis) != common_dim)
if let Some(a) = arrays.iter().find_map(|a|
if a.raw_dim().remove_axis(axis) != common_dim {
Some(a)
} else {
None
})
{
return Err(from_kind(ErrorKind::IncompatibleShape));
return Err(ShapeError::incompatible_shapes(&common_dim, &a.dim));
}

let stacked_dim = arrays.iter().fold(0, |acc, a| acc + a.len_of(axis));
Expand Down Expand Up @@ -143,17 +148,20 @@ where
D::Larger: RemoveAxis,
{
if arrays.is_empty() {
// TODO More specific error for empty input not supported
return Err(from_kind(ErrorKind::Unsupported));
}
let common_dim = arrays[0].raw_dim();
// Avoid panic on `insert_axis` call, return an Err instead of it.
if axis.index() > common_dim.ndim() {
return Err(from_kind(ErrorKind::OutOfBounds));
return Err(ShapeError::invalid_axis(common_dim.ndim(), axis.index()));
}
let mut res_dim = common_dim.insert_axis(axis);

if arrays.iter().any(|a| a.raw_dim() != common_dim) {
return Err(from_kind(ErrorKind::IncompatibleShape));
if let Some(array) = arrays.iter().find_map(|array| if !array.equal_dim(&common_dim) {
Some(array)
} else { None }) {
return Err(ShapeError::incompatible_shapes(&common_dim, &array.dim));
}

res_dim.set_axis(axis, arrays.len());
Expand Down