Use more `Borrow`s

[tafia]

There are many places where copies are done (via select_rows for instance) when a simple borrow would have been enough.

We could probably save some memory by:

• implementing Borrow and AsRef between Matrix and MatrixSlice(Mut) (they make sense I think)
• having more generic traits that require Borrow<T> or AsRef<T> instead of &T

Example with Optimizable:

        /// Trait for models which can be gradient-optimized.
        pub trait Optimizable {
            /// The input data type to the model.
            type Inputs;
            /// The target data type to the model.
            type Targets;
            /// Compute the gradient for the model.
            fn compute_grad(&self,
                            params: &[f64],
                            inputs: Borrow<Self::Inputs>,
                            targets: Borrow<Self::Targets>)
                            -> (f64, Vec<f64>);
        }

Then we could use MatrixSlice instead of select_rows here.

There are probably many examples.

[AtheMathmo]

This is a really, really good suggestion. It will remove a lot of unneeded copying - especially in conjunction with your get_row PR that will be merged soon.

I have yet to use Borrow and AsRef so the differences are lost on me right now. I'll have to do some reading.

I'm happy to work on this issue if you didn't want to.

[tafia]

Please do it! You may come up with new suggestions

On 26 Jul 2016 22:24, "James Lucas" notifications@github.com wrote:

This is a really, really good suggestion. It will remove a lot of unneeded
copying - especially in conjunction with your get_row PR that will be
merged soon.

I have yet to use Borrow and AsRef so the differences are lost on me
right now. I'll have to do some reading.

I'm happy to work on this issue if you didn't want to.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#109 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHAszgp2FsupWbq4G5PF8gzVW3rIOd9sks5qZhikgaJpZM4JVAaY
.

[AtheMathmo]

Ok I'll tackle this one :). I won't be able to tackle it until the weekend (focusing on a presentation for a meetup).

[AtheMathmo]

Added a mirror issue on rulinalg to track the implementation there.

[AtheMathmo]

Closing this out. We've gone with an alternate approach and wont be able to do something like this until custom DSTs land.