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

Fixes related to infix ops PR #4

Merged
merged 3 commits into from
May 27, 2024

Conversation

voltrevo
Copy link
Contributor

@voltrevo voltrevo commented May 15, 2024

Add Operation::IntegerDivide

This is a bit awkward because it's currently the same as Operation::Divide. However I think it's helpful to set up this distinction so that we can treat them differently in the future. In circom Divide means division within the finite field (eg 1/2 is 3 in modulo 5, since 3*2=1 mod 5). Because the available operations depends on the MPC backend, I think we should parameterize sim-circuit over the number system, but I'll make a discussion for this elsewhere.

Discord discussion thread.

Fix input counting bug when constructing graph for Kahn's Algorithm

I was getting CircuitError::CyclicDependencyDetected for this circuit:

pragma circom 2.0.0;

template xEqX() {
    signal input x;
    signal output out;
    
    out <== x == x;
}

component main = xEqX();

(This issue came up because the test I wrote for all infix ops contained several examples like this.)

I discovered the reason for this was that a HashSet is used for the dependencies of each node. This meant that when inserting a duplicate dependency, it was ignored by the hashset but the in-degree was still incremented. Kahn's Algorithm requires that in-degree can be decremented for each edge (including multiple edges from the same input) rather than each distinct dependency.

Copy link
Owner

@brech1 brech1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Thank you

@brech1 brech1 merged commit 115ccb5 into brech1:master May 27, 2024
1 check passed
@voltrevo voltrevo deleted the issue-54-all-infix-ops branch June 3, 2024 07:05
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

Successfully merging this pull request may close these issues.

2 participants