iden3 / circom

zkSnark circuit compiler
GNU General Public License v3.0
1.28k stars 244 forks source link

`parallel` keyword does not work as intended when used at component level #204

Closed ultrainstinct30 closed 11 months ago

ultrainstinct30 commented 1 year ago

Hi. I have been playing around with the parallel keyword a bit and discovered that declaring parallel at component level renders it useless. Please look at the following toy example: version - 2.1.6 doublemultiplier.circom

pragma circom 2.1.6;

template DoubleMultiplier() {
    signal input in[4];
    signal output out[2];

    component multiplier[2];
    for (var i=0; i<2; i++) {
        multiplier[i] = parallel Multiplier();
        multiplier[i].in1 <== in[i*2];
        multiplier[i].in2 <== in[i*2+1];
        out[i] <== multiplier[i].out;
    }
}

template Multiplier() {
    signal input in1;
    signal input in2;
    signal output out;

    out <== in1 * in2;
}

component main = DoubleMultiplier();

Three build folder for the following cases:

After building with circom, running diff -r for build_non_parallel and build_component_parallel produces an empty output, whereas running diff -r for build_non_parallel and build_template_parallel produces non empty output, the following:

diff -r build_non_parallel/doublemultiplier_cpp/Makefile build_template_parallel/doublemultiplier_cpp/Makefile
22c22
<       $(CC) -o doublemultiplier *.o -lgmp 
---
>       $(CC) -o doublemultiplier *.o -lgmp -pthread
diff -r build_non_parallel/doublemultiplier_cpp/doublemultiplier.cpp build_template_parallel/doublemultiplier_cpp/doublemultiplier.cpp
6,7c6,7
< void Multiplier_0_create(uint soffset,uint coffset,Circom_CalcWit* ctx,std::string componentName,uint componentFather);
< void Multiplier_0_run(uint ctx_index,Circom_CalcWit* ctx);
---
> void Multiplier_0_create_parallel(uint soffset,uint coffset,Circom_CalcWit* ctx,std::string componentName,uint componentFather);
...a lot of more lines
clararod9 commented 11 months ago

Hi!

Thank you for reporting this issue, we have solved it in our last commits. Let us know if the current implementation has the expected behavior.