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

Implement num_traits::identities::{One, Zero} for Variable #133

Open
avhz opened this issue Sep 24, 2023 · 12 comments
Open

Implement num_traits::identities::{One, Zero} for Variable #133

avhz opened this issue Sep 24, 2023 · 12 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@avhz
Copy link
Owner

avhz commented Sep 24, 2023

I need to find a reasonable way to implement the num_traits::identities::{One, Zero} traits for the Variable type in RustQuant::autodiff.

The traits are here

@avhz avhz added enhancement New feature or request help wanted Extra attention is needed labels Sep 24, 2023
@matormentor
Copy link
Contributor

matormentor commented Nov 3, 2024

Since Variable operations happen arround variable.value() which itself is an f64 I suggest:

  1. Return a Variable with an address to an empy graph, vertex index 0 and values 1, 0 to comply with backpropagation identity.
  2. Create the variable with defaults and the value to be 1, 0.

For both of this cases we would either need to own Graph instead of having a reference or to have it passed as a parameter of the one function which in turn makes the use of the num_traits not possible.

@avhz
Copy link
Owner Author

avhz commented Nov 7, 2024

Hi thanks for the suggestions.

Point 2 is not possible with the current implementation because we require a reference to a shared graph, or could make a new graph in the zero() method which doesn't make sense.

I am not sure what you mean by Point 1. Could you describe what you have in mind a bit more ?

@matormentor
Copy link
Contributor

matormentor commented Nov 8, 2024

Both num_traits::identities::{One, Zero} require a function one() -> Self or zero() -> Self that take no arguments for the initialization of variable. Because:

pub struct Variable<'v> {
    pub graph: &'v Graph,
    pub index: usize,
    pub value: f64,
}

we need a reference, hence a borrow of a graph when creating the One, Zero traits.

Thus, maybe a declaration of an empty graph as a global lazy_static! or static variable may be needed in order to properly initialize the one() -> Self and zero() -> Self functions as:

impl<'v> One for Variable<'v> {
	fn one() -> Self {
		Variable{
			graph: GLOBAL_GRAPH,
			index: Zero::zero(),
			value: One::one()
		}
	}
}

since a default variable may have an index equal to zero and the values are thus given by 0, 1 depending on the trait Zero, One respectively.

Let me know what your thoughts are.

@avhz
Copy link
Owner Author

avhz commented Nov 8, 2024

I have thought about a global graph, but I am not sure because either:

  • It means other Variables will not be on the same graph, so we cannot perform operations on them.
  • Or we use the global graph for all variables, but I believe this would mean that users are limited to one graph per program.

@matormentor
Copy link
Contributor

matormentor commented Nov 11, 2024

I have implemented the functions using a Static empty graph. And added in the Variable struct a setter for the graph. Then, if we want to operate with the Zero, One traits we would create it as:

let var = Variable{
                    graph: &my_graph,
                    ...}
let zero = Variable::zero()
zero.set_graph(&my_graph)
// Operate with zero and var

Let me know what you think

Edit: I omitted the part that there was the need of changing the Graphs to a thread safe construct from

pub struct Graph {
    /// Vector containing the vertices in the Wengert List.
    pub vertices: RefCell<Vec<Vertex>> to -> Arc<RwLock<Vec<Vertex>>>,
}

@avhz
Copy link
Owner Author

avhz commented Nov 11, 2024

Thanks for the example :)

Are users able to create an arbitrary number of graphs, or is it a single global static ?

And does this push any cloning/locking/unlocking onto the end user ?

@matormentor
Copy link
Contributor

  1. Users can create an arbitrarily number of graphs represented by this test:
	#[test]
	fn test_multiple_graphs_different_address() {
		let g1 = Graph::new();
		let g2 = Graph::new();

		let mut one1 = Variable::one();
		one1.set_graph(&g1);
		let mut one2 = Variable::one();
		one2.set_graph(&g2);
		let mut zero1 = Variable::zero();
		zero1.set_graph(&g1);
		let mut zero2 = Variable::zero();
		zero2.set_graph(&g2);

		println!("{:p} {:p}", zero1.graph, zero2.graph);
		println!("{:p} {:p}", one1.graph, one2.graph);
		assert!(std::ptr::addr_eq(zero1.graph, one1.graph));
		assert!(std::ptr::addr_eq(zero2.graph, one2.graph));
		assert!(!std::ptr::addr_eq(zero1.graph, zero2.graph));
		assert!(!std::ptr::addr_eq(one1.graph, one2.graph));
	}

and its output
image

  1. It is needed for the graph.vertices() to call read() and write() functions to manipulate them given the RwLock which returns a Result but other functionality remains unchanged.

@avhz
Copy link
Owner Author

avhz commented Nov 17, 2024

I'd be interested to see the code, do you have a branch or repo you can link me to ?

@matormentor
Copy link
Contributor

matormentor commented Nov 18, 2024

Here is the branch forked from the RustQuant repository.
Edit: Please see the last commit contain all changes done from the last state

@avhz
Copy link
Owner Author

avhz commented Nov 19, 2024

With the set_graph() logic, we don't need a global static graph to use in Zero and One, we can just call graph: Graph::new() and then set the graph to whichever graph we are currently using.

It's not really meaningful though, since the index on the zero and one variables will not make sense, because there can be more than one variable with index 0 for example, so operations on these variables will be garbled.
If you use the Graphviz plotting I think this would show what I mean.

I could be mistaken so correct me if I'm wrong, but I think it does not solve the problem of needing Zero/One for filling nalgebra matrices or ndarrays with Variables.

@matormentor
Copy link
Contributor

W.l.o.g. lets talk about the One case.

To answer the first question: we still have the need to use a static graph since the One trait needs the one() function to return a Variable with an address to a graph that outlives the function itself. So it is needed to define the trait, even if after it, it wont be used because another graph may be set.

The other option I can think of is to create a custom function one(graph: &Graph, index: usize) that generates "ones" variables. But it is not compliant with the nums_traits because it has additional parameters.

For the second note. I agree that there will be more than one variable with index 0. But is also true that without additional parameters there is no way we can infer the index of a new variable. (although maybe a global counter could work). e.g

	fn zero() -> Self {
		*STATIC_GRAPH.push(Arity::Nullary, &[], &[]);
		Variable{
			graph: &*STATIC_GRAPH,
			index: *STATIC_GRAPH.len(),
			value: Zero::zero()
		}
	}

But it is true that it does not solve the problem immediately.

@avhz
Copy link
Owner Author

avhz commented Nov 22, 2024

Seems I was too tired to recall Rust 101, thanks for pointing that out.

I have not taken a proper look at this module for some time, but I am now interested again so I will have a look over the weekend and see if I can think of anything that might work.

I appreciate the interest :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants