Move to `ArrayRef` instead of `ArrayBase`

[akern40]

To take full advantage of ndarray 0.17, it would be good to move the library to the new ArrayRef reference type. While this is mostly simple changes, it will take some time to break it into multiple reviewable PRs.

[akern40]

@Dirreke I've got the code for this just about ready. There's one place where I haven't been able to change over to ArrayRef: generalized eigenv*. This is because those are implemented for (ArrayBase<S, Ix2>, ArrayBase<S2, Ix2>); I can't use (ArrayRef<A, Ix2>, ArrayRef<A, Ix2>) for two reasons. The first is that ArrayRef is unsized, and so it can only be the last member of a tuple. The second is that the ergonomics wouldn't be great, since we wouldn't get auto-deref.

As I see it, there are two options:

1. Leave them implemented on ArrayBase. Users can always use something like (a_ref1.view(), a_ref2.view())
2. Create some sort of struct that would help with the ergonomics. Something like EigGen(&a_ref1, &a_ref2).compute()

We could always do both, as well. Curious to hear your thoughts!

[Dirreke]

Hi, Thanks for the work.

This is indeed a tough choice. IMO, I believe we could adopt both approaches and make a gradual transition to the second option. Specifically, we can retain the first option for now, but mark the eig_generized-like functions in Option 1 as deprecated. This will allow us to guide users towards adopting the new approach over time, without breaking existing workflows.

By deprecating the old usage, we can signal that Option 2 (e.g., using EigGen) is the preferred method moving forward while ensuring backward compatibility for existing users.

What do you think?

In addition, do you think we need to yank 0.18.0 and then publish 0.18.1 to support this breaking change? Or just publish 0.19.0?

[akern40]

I'm good with that approach! I think it strikes a nice balance between the two options. Two additional design choices to get your input on: should we use a struct or just a function? And if we use a struct, what should it be named?

As for versioning, I think we could just release 0.18.1. The nice thing about the array reference type is that it is almost completely backwards compatible; in the case of this crate, I haven't had to touch a single test or benchmark. Deprecating doesn't require a minor version bump (I don't think) and the new reference type implementations don't introduce breaking changes, either.

[Dirreke]

I’m comfortable with both. Personally, I prefer using a struct for no particular reason. For names, I think EigGen is a good choice. Do you have any other ideas?

[akern40]

I took a slightly different approach; let me know what you think of this:

/// Turn arrays, references to arrays, and [`ArrayRef`]s into owned arrays
pub trait MaybeOwnedMatrix {
    type Elem;

    /// Convert into an owned array, cloning only when necessary.
    fn into_owned(self) -> Array2<Self::Elem>;
}

impl<S> MaybeOwnedMatrix for ArrayBase<S, Ix2>
where
    S: Data,
    S::Elem: Clone,
{
    type Elem = S::Elem;

    fn into_owned(self) -> Array2<S::Elem> {
        ArrayBase::into_owned(self)
    }
}

impl<S> MaybeOwnedMatrix for &ArrayBase<S, Ix2>
where
    S: Data,
    S::Elem: Clone,
{
    type Elem = S::Elem;

    fn into_owned(self) -> Array2<S::Elem> {
        self.to_owned()
    }
}

impl<A> MaybeOwnedMatrix for &ArrayRef2<A>
where
    A: Clone,
{
    type Elem = A;

    fn into_owned(self) -> Array2<A> {
        self.to_owned()
    }
}

impl<T1, T2> EigGeneralized for (T1, T2)
where
    T1: MaybeOwnedMatrix,
    T1::Elem: Lapack + Scalar,
    T2: MaybeOwnedMatrix<Elem = T1::Elem>,
{
    type EigVal = Array1<GeneralizedEigenvalue<<T1::Elem as Scalar>::Complex>>;
    type EigVec = Array2<<T1::Elem as Scalar>::Complex>;
    type Real = <T1::Elem as Scalar>::Real;

    fn eig_generalized(
        self,
        thresh_opt: Option<Self::Real>,
    ) -> Result<(Self::EigVal, Self::EigVec)> {
        let (mut a, mut b) = (self.0.into_owned(), self.1.into_owned());
        let layout = a.square_layout()?;
        let (s, t) = T1::Elem::eig_generalized(
            true,
            layout,
            a.as_allocated_mut()?,
            b.as_allocated_mut()?,
            thresh_opt,
        )?;
        let n = layout.len() as usize;
        Ok((
            ArrayBase::from(s),
            Array2::from_shape_vec((n, n).f(), t).unwrap(),
        ))
    }
}

[Dirreke]

Looks Great. I like this approach.