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

Migrate over to num crate #80

Open
Uzaaft opened this issue Jul 23, 2023 · 6 comments
Open

Migrate over to num crate #80

Uzaaft opened this issue Jul 23, 2023 · 6 comments

Comments

@Uzaaft
Copy link
Contributor

Uzaaft commented Jul 23, 2023

There's a lot of code where methods are implemented on different type of numbers(f64, i64, u64, f32, etc...)
Example of file: https://github.com/avhz/RustQuant/blob/main/src/utilities/sequence.rs

Wouldn't it make sense to reduce the amount of code by adding some library, like https://github.com/rust-num/num, that has a collection of numeric types and traits which could be used.

@avhz
Copy link
Owner

avhz commented Jul 23, 2023

Yes I've wanted to use the num-traits for a while, especially for the Variable type in the autodiff module, but that seems difficult or impossible currently.

If you want to use num in another part like for the sequences go ahead, that would be awesome, thank you!
And if you know how to impl Zero and One for Variable that would be even better !

@Uzaaft
Copy link
Contributor Author

Uzaaft commented Jul 23, 2023

@Uzaaft
Copy link
Contributor Author

Uzaaft commented Jul 23, 2023

Disclaimer: I'm not a math wiz at all. So my understanding might be lacking.
For the Variablestruct, isn't it just to do:

use num::traits::{One, Zero};

// other parts of your code...

impl<'v> Zero for Variable<'v> {
    fn zero() -> Self {
        Variable {
            graph: &Graph::new(), // you need to provide a way to get a default graph
            index: 0, // assuming 0 is a sensible default index
            value: 0.0,
        }
    }

    fn is_zero(&self) -> bool {
        self.value == 0.0
    }
}

impl<'v> One for Variable<'v> {
    fn one() -> Self {
        Variable {
            graph: &Graph::new(), // you need to provide a way to get a default graph
            index: 0, // assuming 0 is a sensible default index
            value: 1.0,
        }
    }

    fn is_one(&self) -> bool {
        self.value == 1.0
    }
}

@avhz
Copy link
Owner

avhz commented Jul 23, 2023

This won't work since multiple Variables won't refer to the same Graph so performing operations on them such as x + y won't work.

The reason it is needed is to enable nalgebra matrices to contain Variable. See #63, #64, #72.

@Autoparallel
Copy link
Contributor

How is this @Uzaaft? It seems like a good idea. Any way I can help?

@Autoparallel
Copy link
Contributor

Checking in again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants