Skip to content

implement creating SliceArg from arbitrary Dimension #909

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

implement creating SliceArg from arbitrary Dimension #909

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
085a319
implement creating SliceArg from arbitrary Dimension
SparrowLii Feb 3, 2021
9d76f13
use slice_info_from in dimension_trait.rs instead
SparrowLii Feb 4, 2021
03c3791
Transfer function into SliceInfo; Use Result as the return value
SparrowLii Feb 5, 2021
94aab16
Merge branch 'master' into slicearg
SparrowLii Feb 5, 2021
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
50 changes: 49 additions & 1 deletion src/dimension/dimension_trait.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@
use std::fmt::Debug;
use std::ops::{Add, AddAssign, Mul, MulAssign, Sub, SubAssign};
use std::ops::{Index, IndexMut};
use std::convert::TryInto;
use alloc::vec::Vec;

use super::axes_of;
use super::conversion::Convert;
use super::{stride_offset, stride_offset_checked};
use crate::itertools::{enumerate, zip};
use crate::Axis;
use crate::{Axis, SliceInfo};
use crate::IntoDimension;
use crate::RemoveAxis;
use crate::{ArrayView1, ArrayViewMut1};
Expand Down Expand Up @@ -78,6 +79,34 @@ pub trait Dimension:
/// Next larger dimension
type Larger: Dimension + RemoveAxis;

/// Convert index to &Self::SliceArg. Make sure that length of index
/// consists with Self::NDIM(if it exists).
///
/// Panics if conversion failed.
#[doc(hidden)]
fn slice_arg_from(index: &[SliceOrIndex]) -> &Self::SliceArg ;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure if this should be a method on Dimension. It could be, it's a bit wonky? This is a hidden method so it is then not intended to be public user-facing API.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should not be listed as the first method in the Dimension trait. It is not the most important method in the trait.

Copy link
Member

@jturner314 jturner314 Feb 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do something like this:

use std::convert::{TryFrom, TryInto};

pub trait TryFromRef<T: ?Sized> {
    type Error;
    fn try_from_ref(orig: &T) -> Result<&Self, Self::Error>;
}

impl<T: ?Sized, U: ?Sized> TryFromRef<T> for U
where
    for<'a> &'a U: TryFrom<&'a T>,
{
    // Once Rust has const generics, we can implement `TryFromRef` separately
    // for `[SliceOrIndex]` and `[SliceOrIndex; N]`, and use
    // `TryFromSliceError` for the `[SliceOrIndex; N]` case.
    // Actually, we only really need to implement this trait for `N <= 6`, so we until
    // Rust has const generics, we could have separate impls for each size.
    type Error = ();

    fn try_from_ref(orig: &T) -> Result<&Self, Self::Error> {
        orig.try_into().map_err(|_| ())
    }
}

pub trait Dimension {
    ...
    type SliceArg: ?Sized + AsRef<[SliceOrIndex]> + TryFromRef<[SliceOrIndex]>;
    ...
}

although this is pretty awkward too.


/// Convert &SliceInfo<AsRef<[SliceOrIndex]>, Do> to &SliceInfo<D::SliceArg, Do>.
/// Generate SliceArg of any dimension via this method.
///
/// Panics if conversion failed.
#[doc(hidden)]
fn slice_info_from<T, Do>(indices: &T) -> &SliceInfo<Self::SliceArg, Do>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't correct, because the Do here is unconstrained and is not checked. Instead, we should add a method to SliceInfo:

impl<T: ?Sized, D> SliceInfo<T, D>
where
    T: AsRef<[SliceOrIndex]>,
    D: Dimension,
{
    pub fn for_dimensionality<E>(&self) -> Result<&SliceInfo<E::SliceArg, D>, ShapeError>
    where
        E: Dimension,
    {
        ...
    }
}

which makes sure that the D is correctly preserved.

Copy link
Member

@jturner314 jturner314 Feb 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what it's worth, we could do this instead to avoid the need for Dimension::slice_arg_from:

impl<T: ?Sized, D> SliceInfo<T, D>
where
    T: AsRef<[SliceOrIndex]>,
    D: Dimension,
{
    pub fn for_dimensionality<'a, E>(&'a self) -> &'a SliceInfo<E::SliceArg, D>
    where
        E: Dimension,
        &'a E::SliceArg: TryFrom<&'a [SliceOrIndex]>,
    {
        ...
    }
}

but that would make it awkward to use in generic functions because users would have to add the &'a E::SliceArg: TryFrom<&'a [SliceOrIndex]> bound to their functions.

Copy link
Contributor Author

@SparrowLii SparrowLii Feb 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, function should be put in SliceInfo. But I think it seems difficult to move slice_arg_from out of Dimension. try_into() needs to ensure that SliceArg implements From<&[SliceOrIndex]>. I think we should avoid adding additional restrictions on SliceArg.

where
T: AsRef<[SliceOrIndex]>,
Do: Dimension,
{
let arg_ref = Self::slice_arg_from(indices.as_ref());
unsafe {
// This is okay because the only non-zero-sized member of
// `SliceInfo` is `indices`, so `&SliceInfo<[SliceOrIndex], D>`
// should have the same bitwise representation as
// `&[SliceOrIndex]`.
&*(arg_ref as *const Self::SliceArg
as *const SliceInfo<Self::SliceArg, Do>)
}
}

/// Returns the number of dimensions (number of axes).
fn ndim(&self) -> usize;

Expand Down Expand Up @@ -398,6 +427,10 @@ impl Dimension for Dim<[Ix; 0]> {
type Pattern = ();
type Smaller = Self;
type Larger = Ix1;
#[inline]
fn slice_arg_from(index: &[SliceOrIndex]) -> &Self::SliceArg {
index.try_into().unwrap()
}
// empty product is 1 -> size is 1
#[inline]
fn ndim(&self) -> usize {
Expand Down Expand Up @@ -442,6 +475,9 @@ impl Dimension for Dim<[Ix; 1]> {
type Pattern = Ix;
type Smaller = Ix0;
type Larger = Ix2;
fn slice_arg_from(index: &[SliceOrIndex]) -> &Self::SliceArg {
index.try_into().unwrap()
}
#[inline]
fn ndim(&self) -> usize {
1
Expand Down Expand Up @@ -558,6 +594,9 @@ impl Dimension for Dim<[Ix; 2]> {
type Pattern = (Ix, Ix);
type Smaller = Ix1;
type Larger = Ix3;
fn slice_arg_from(index: &[SliceOrIndex]) -> &Self::SliceArg {
index.try_into().unwrap()
}
#[inline]
fn ndim(&self) -> usize {
2
Expand Down Expand Up @@ -715,6 +754,9 @@ impl Dimension for Dim<[Ix; 3]> {
type Pattern = (Ix, Ix, Ix);
type Smaller = Ix2;
type Larger = Ix4;
fn slice_arg_from(index: &[SliceOrIndex]) -> &Self::SliceArg {
index.try_into().unwrap()
}
#[inline]
fn ndim(&self) -> usize {
3
Expand Down Expand Up @@ -838,6 +880,9 @@ macro_rules! large_dim {
type Pattern = $pattern;
type Smaller = Dim<[Ix; $n - 1]>;
type Larger = $larger;
fn slice_arg_from(index: &[SliceOrIndex]) -> &Self::SliceArg {
index.try_into().unwrap()
}
#[inline]
fn ndim(&self) -> usize { $n }
#[inline]
Expand Down Expand Up @@ -889,6 +934,9 @@ impl Dimension for IxDyn {
type Pattern = Self;
type Smaller = Self;
type Larger = Self;
fn slice_arg_from(index: &[SliceOrIndex]) -> &Self::SliceArg {
index
}
#[inline]
fn ndim(&self) -> usize {
self.ix().len()
Expand Down
22 changes: 22 additions & 0 deletions tests/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,28 @@ fn test_slice_dyninput_vec_dyn() {
arr.view().slice_collapse(info.as_ref());
}

#[test]
fn test_slice_arg() {
fn use_arg_map<Sh, D>(shape: Sh, f: impl Fn(&usize) -> SliceOrIndex, shape2: D)
where
Sh: ShapeBuilder,
D: Dimension,
{
let shape = shape.into_shape();
let mut x = Array::from_elem(shape, 0);
let indices = x.shape().iter().map(f).collect::<Vec<_>>();
let s = x.slice_mut(
<Sh::Dim as Dimension>::slice_info_from::<_, Sh::Dim>(&indices)
);
let s2 = shape2.slice();
assert_eq!(s.shape(), s2)
}
use_arg_map(0,|x| SliceOrIndex::from(*x/2..*x),Dim([0]));
use_arg_map((2, 4, 8),|x| SliceOrIndex::from(*x/2..*x),Dim([1, 2, 4]));
use_arg_map(vec![3, 6, 9],|x| SliceOrIndex::from(*x/3..*x/2),Dim([0, 1, 1]));
use_arg_map(vec![1, 2, 3, 4, 5, 6, 7], |x| SliceOrIndex::from(x-1), Dim([]));
}

#[test]
fn test_slice_with_subview() {
let mut arr = ArcArray::<usize, _>::zeros((3, 5, 4));
Expand Down