feat: generic circuit struct #123

brech1 · 2024-05-02T21:48:59Z

Description

This PR aims to introduce a new generic circuit struct:

pub struct Circuit<T> {
    gates: Vec<T>,
}

The Circuit is a collection of gates. Since for the execution of these gates is important that these form a directed acyclic graph (DAG), the correct way of building this circuit is using the CircuitBuilder

pub struct CircuitBuilder<T>
where
    T: Component,
{
    gates: Vec<T>,
    input_map: HashMap<usize, usize>,
}

The CircuitBuilder sort the gates topologically, this means in the order they should be executed taking dependencies into account, and ensures that they form a DAG.

Here is our only constrain for the gates, they have to implement the Component trait, this means that we should be able to fetch their inputs and outputs. This is strictly necessary to sort the gates.

pub trait Component {
    fn get_inputs(&self) -> Vec<usize>;
    fn get_outputs(&self) -> Vec<usize>;
}

The Component is our only trait in the model module at the moment, aimed at storing defining traits.

Example

A sample of how to use this can be found in the tests directory.

let mut circuit_builder = CircuitBuilder::<BinaryGate>::new();

let gate1 = BinaryGate {
    inputs: vec![0, 1],
    output: 2,
};
let gate2 = BinaryGate {
    inputs: vec![3, 4],
    output: 5,
};

circuit_builder
    .add_gate(gate1)
    .add_gate(gate2);

let circuit = circuit_builder.build();

brech1 · 2024-05-06T15:32:39Z

The binary module is just an example of how to use the generic structs with a basic binary circuit. So can be seen as temporary to review the pr or be left aside.
The way we decide to deal with declaring inputs and outputs of the circuit, or this kind of "builder api" can shape the interface or even remove it completely. Current implementation is just a basic one to test the structure. In this point perhaps we could prioritize to make things easier to deal with mpz or adapt to the old structure as much as possible.

brech1 · 2024-05-12T13:46:10Z

The model has been updated with few key changes:

Both circuit and gates can be modeled as:
- components: blocks with inputs and outputs
- executables: can perform external operations on memory

A key change is that the execution is defined externally on both, by part of an Executor.

This means that each structure can perform multiple operations with the same configuration.

sinui0

I'm not sure I'm convinced of the execution abstractions here. For now I think it would be easiest to just stick to developing the Circuit type and have it enforce the invariants we discussed previously, eg topological sort etc. We can add execution in a follow up PR.

sinui0

Nice, this will be a much more flexible base for our circuit models!

Some requests 🙏

sinui0 · 2024-06-04T03:55:44Z

crates/mpz-circuits-generic/src/circuit.rs

+
+/// Circuit errors.
+#[derive(Debug, Error, PartialEq, Eq)]
+pub enum CircuitError {


Suggested change

pub enum CircuitError {

pub enum CircuitBuilderError {

sinui0 · 2024-06-04T03:56:21Z

crates/mpz-circuits-generic/src/lib.rs

+pub mod circuit;
+pub mod model;


Let's make these private and export needed types

sinui0 · 2024-06-04T03:56:42Z

crates/mpz-circuits-generic/tests/sorting.rs

+}
+
+#[cfg(test)]
+mod tests {


Can we add a couple test cases?

Unordered gates

Disconnected gate

Node ids are contiguous

Node ids are ordered: { input nodes } .. { output nodes }

I added the disconnected gate test, but I'm confused about testing the node ids, in which way they should be contiguous or ordered? The order is based on that the gates should be ready to be processed, so node ids are not always contiguous or ordered

The order is based on that the gates should be ready to be processed

Exactly, so the node id should correspond to the gate position. For example:

assert!(gates.windows(2).map(|window| window[1].out.0 == window[0].out.0 + 1).all());

This asserts the nodes are ordered and contiguous.

sinui0 · 2024-06-04T04:04:35Z

crates/mpz-circuits-generic/src/model.rs

+/// A `Component` trait that represents a block with inputs and outputs.
+pub trait Component {
+    /// Returns the input node indices.
+    fn get_inputs(&self) -> Vec<usize>;


We should return impl Iterator<Item = &Node> so that it's possible for the ids to be stack allocated, ie [Node; 2], or some other data structure. Otherwise every call to this function is going to incur a heap allocation.

sinui0 · 2024-06-04T04:06:58Z

crates/mpz-circuits-generic/src/model.rs

Can we add a Node newtype, as we currently have in mpz-circuits. Gives us better typing. Perhaps we should also switch it from usize to u32 so the circuit isn't needlessly twice the size on 64 bit archs.

I think that adding a Node type is not strictly necessary so I wouldn't implement it in the model. Don't see how much of an improvement this extra typing could provide on our level, perhaps implementers could add this layer themselves.

The input or output is intended to be a position on a linear memory array so it will be usize in the end, on 64-bit architectures you'll have to complete the address to access an index.

But both are good suggestions, I could be missing something!

Don't see how much of an improvement this extra typing could provide on our level

It's all about the public API: What do you want to commit to? Using usize commits to that underlying representation, whereas a Node(u32) type allows us to constrain the data type and the traits which a node implements. For example why would a node implement Add? usize does. Do we want to allow it to be mutated? How might that affect certain invariants we have elsewhere? This is less an issue for private types, but this is explicitly part of the crates pub api.

The input or output is intended to be a position on a linear memory array so it will be usize in the end

Yes it will be used to index into linear memory, but a Vec<u32> is half the size of a Vec<u64>, and hence a Vec<Gate> will be half the size if Node is backed by a u32 instead of u64 on 64-bit archs.

sinui0 · 2024-06-07T16:59:00Z

crates/mpz-circuits-generic/tests/sorting.rs

+        assert!(
+            circuit.is_ok(),
+            "Failed to build circuit: {:?}",
+            circuit.err()
+        );


This should be an error, no? A circuit should not have disconnected gates

sinui0 · 2024-06-07T17:07:47Z

crates/mpz-circuits-generic/tests/sorting.rs

+}
+
+#[cfg(test)]
+mod tests {


The order is based on that the gates should be ready to be processed

Exactly, so the node id should correspond to the gate position. For example:

assert!(gates.windows(2).map(|window| window[1].out.0 == window[0].out.0 + 1).all());

This asserts the nodes are ordered and contiguous.

sinui0 · 2024-06-07T17:08:14Z

crates/mpz-circuits-generic/src/model.rs

+
+/// A circuit node, holds a generic type identifier.
+#[derive(Debug, Eq, PartialEq)]
+pub struct Node<T> {


Why is this generic?

brech1 · 2024-06-10T23:54:48Z

Closing for #156

brech1 changed the base branch from dev to threading-refactor May 2, 2024 22:33

brech1 force-pushed the feat/generic-circuit branch from 740a715 to 98cd0b9 Compare May 5, 2024 05:02

brech1 added 5 commits May 27, 2024 19:46

feat: generic circuit

12eb964

docs: circuit module title

242a1c9

docs: fix comments

6b76ffc

fix: model

df4b074

add: example binary impl

599f66c

brech1 force-pushed the feat/generic-circuit branch from 3ba9966 to 599f66c Compare May 27, 2024 17:47

sinui0 reviewed May 27, 2024

View reviewed changes

brech1 added 3 commits May 28, 2024 11:27

remove execution

8947554

implement sorting

db13e8e

fmt and fix tests

e1cea18

brech1 marked this pull request as ready for review May 31, 2024 15:22

sinui0 requested changes Jun 4, 2024

View reviewed changes

implement required changes

cf4f3a5

sinui0 reviewed Jun 7, 2024

View reviewed changes

brech1 added 2 commits June 7, 2024 17:58

update

3ac77dd

remove generic node

8ed2ac2

brech1 closed this Jun 10, 2024

brech1 deleted the feat/generic-circuit branch December 4, 2024 20:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: generic circuit struct #123

feat: generic circuit struct #123

brech1 commented May 2, 2024 •

edited

Loading

brech1 commented May 6, 2024 •

edited

Loading

brech1 commented May 12, 2024 •

edited

Loading

sinui0 left a comment

sinui0 left a comment

sinui0 Jun 4, 2024

sinui0 Jun 4, 2024

sinui0 Jun 4, 2024

brech1 Jun 7, 2024

sinui0 Jun 7, 2024

sinui0 Jun 4, 2024 •

edited

Loading

sinui0 Jun 4, 2024

brech1 Jun 5, 2024

sinui0 Jun 5, 2024

sinui0 Jun 7, 2024

sinui0 Jun 7, 2024

sinui0 Jun 7, 2024

brech1 commented Jun 10, 2024

feat: generic circuit struct #123

feat: generic circuit struct #123

Conversation

brech1 commented May 2, 2024 • edited Loading

Description

Example

brech1 commented May 6, 2024 • edited Loading

brech1 commented May 12, 2024 • edited Loading

sinui0 left a comment

Choose a reason for hiding this comment

sinui0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sinui0 Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brech1 commented Jun 10, 2024

brech1 commented May 2, 2024 •

edited

Loading

brech1 commented May 6, 2024 •

edited

Loading

brech1 commented May 12, 2024 •

edited

Loading

sinui0 Jun 4, 2024 •

edited

Loading