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

autodiff: add support for nalgebra matrices/vectors. #64

Open
avhz opened this issue Jul 3, 2023 · 10 comments
Open

autodiff: add support for nalgebra matrices/vectors. #64

avhz opened this issue Jul 3, 2023 · 10 comments
Assignees
Labels
difficult Hard or lengthy task. enhancement New feature or request

Comments

@avhz
Copy link
Owner

avhz commented Jul 3, 2023

No description provided.

@avhz avhz added enhancement New feature or request difficult Hard or lengthy task. labels Jul 3, 2023
@avhz avhz self-assigned this Jul 4, 2023
@WenqingZong
Copy link

WenqingZong commented Jul 6, 2023

Opps, just realised there is an issue for it and you assigned to yourself.

This is my blocker of #63 and I'm trying to fix it as well.

nalgebra requires its matrix element to be 'static, so it's not easy to create a matrix full of our Variable structure as it contains a reference to Graph. Currently I'm refactoring Variable structure to use Rc<Graph> instead, but it leads me to other troublesome problems.

You can see my progress in main...WenqingZong:RustQuant:wenqing/neural-networks Currently the code doesn't compile due to an error which I'm not sure how to fix, was meant to ask for your help but found you are working on it as well.

The Problem:
In Variable.rs, after changing graph: &Graph to graph: Rc<Graph>, the graph.rs/var() method failed to compile as creating a Variable would take the ownership of a Graph instance.

@avhz
Copy link
Owner Author

avhz commented Jul 7, 2023

If I recall correctly, using Rc means that the user needs to do things like:

// Original
let z = x + y;

// Using `Rc`
let z = x.clone() + y.clone();

Which I think is not user friendly. Is this the problem you had ?

@WenqingZong
Copy link

If I recall correctly, using Rc means that the user needs to do things like:

// Original
let z = x + y;

// Using `Rc`
let z = x.clone() + y.clone();

Which I think is not user friendly. Is this the problem you had ?

Hmmm, ok, forget about the Rc solution then. But again the current Variable structure cannot be used to populate an nalgebra matrix as it has lifetime 'v and... it's really blocking neural net implementation.

@avhz
Copy link
Owner Author

avhz commented Jul 8, 2023

I could try ndarray and see if it is the same problem?

@WenqingZong
Copy link

I could try ndarray and see if it is the same problem?

superb

@avhz
Copy link
Owner Author

avhz commented Jul 9, 2023

I tried briefly this morning using ndarray and it seems they can hold custom types just fine, however I think we would need to implement a matmul etc ourselves to make it useful.

I can maybe take a closer look tomorrow.

@avhz
Copy link
Owner Author

avhz commented Jul 9, 2023

The trouble with ndarray is for matrix multiplication the type needs to impl Dot which I can't do since it's external.

Component-wise multiplication works fine, but can't do matrix multiplication I think.

#[cfg(test)]
mod test_ndarray {

    #[test]
    fn test_component_mul() {
        let g = crate::autodiff::Graph::new();

        let (a, b, c, d) = (g.var(1.), g.var(2.), g.var(3.), g.var(4.));
        let (e, f, g, h) = (g.var(5.), g.var(6.), g.var(7.), g.var(8.));

        // a = [[1, 2],
        //      [3, 4]]
        // b = [[5, 6],
        //      [7, 8]]
        let a = ndarray::array![[a, b], [c, d]];
        let b = ndarray::array![[e, f], [g, h]];

        // COMPONENT-WISE MULTIPLICATION
        // c = [[5 , 12],
        //      [21, 32]]
        let c = a * b;                                          // <--- This works fine.
        let c_values = c.map(|x| x.value);
        let c_expected = ndarray::array![[5., 12.], [21., 32.]];

        // MATRIX MULTIPLICATION
        // let dot = a.dot(&b);                                 // <--- This does not work.

        assert_eq!(c, c_expected);

        println!("c: {:?}", c);
        println!("c_values: {:?}", c_values);
        println!("c_expected: {:?}", c_expected);
    }
}

@avhz
Copy link
Owner Author

avhz commented Jul 9, 2023

It would work I think if I could impl num::One and num::Zero traits for the Variable type but I don't see how this could be done.

@WenqingZong
Copy link

It would work I think if I could impl num::One and num::Zero traits for the Variable type but I don't see how this could be done.

Would a global, static graph variable solve that?

@avhz
Copy link
Owner Author

avhz commented Jul 9, 2023

I tried that earlier but it required unsafe and then I still got a lifetime issue trying to implement the matrix multiplication.

You can see it in the latest commit in the ndarray.rs file.

@avhz avhz changed the title RustQuant::autodiff: add support for nalgebra matrices/vectors. autodiff: add support for nalgebra matrices/vectors. Jul 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficult Hard or lengthy task. enhancement New feature or request
Projects
No open projects
Status: No status
Development

No branches or pull requests

2 participants