Skip to content

Simplification pass for ACIR#635

Closed
guipublic wants to merge 4 commits intomasterfrom
gd/issue_573
Closed

Simplification pass for ACIR#635
guipublic wants to merge 4 commits intomasterfrom
gd/issue_573

Conversation

@guipublic
Copy link
Copy Markdown
Contributor

Related issue(s)

Resolves #573

Description

Summary of changes

Add a simplification pass which removes gates of the form x=field value, by replacing x by its value in all the other gates.
The simplification is done on the fly, every-time a gate is added to the circuit. If it can be simplified, we update the previous gates using the solved value until nothing is solved. Usually only one pass is needed (so two in total to see that the second one did not solve anything)

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.

@guipublic guipublic requested a review from kevaundray January 13, 2023 13:37
@kevaundray
Copy link
Copy Markdown
Contributor

For reference, why is this done on the fly instead of at the end like the other passes?

(Doing it on the fly seems to have made a lot of the code now need to take a Result)

@guipublic
Copy link
Copy Markdown
Contributor Author

guipublic commented Jan 16, 2023

For reference, why is this done on the fly instead of at the end like the other passes?

This is done in order to minimise the impact of the simplification. If we do it at the end, we need to reprocess the whole circuit again and again until nothing is solved, and the circuit can be big. Here, when we add a gate and if it can be solved, then usually the solved witness is defined by the gate and so we don't need to perform another pass on the circuit.
When the solved witness is not defined by the gate we need to do a full pass over the circuit (but the circuit is not complete so it is smaller) and because all the gates were previously solved, in practice only one is needed. With all the integration tests, there is only one case where more than one pass is needed.

Comment thread crates/acvm/src/simplify.rs Outdated
@kevaundray
Copy link
Copy Markdown
Contributor

There's been quite a bit of merge conflicts on this, that I will resolve

@jfecher
Copy link
Copy Markdown
Contributor

jfecher commented Mar 21, 2023

What is the status on this PR? Do we want to try to fix the conflicts and merge it, restart it, or abandon it?

@guipublic
Copy link
Copy Markdown
Contributor Author

What is the status on this PR? Do we want to try to fix the conflicts and merge it, restart it, or abandon it?

I think the acvm changes are too big for solving the conflicts. I will create another PR (probably we need two now) and then we can abandon this one.

@TomAFrench
Copy link
Copy Markdown
Member

Closing in favour of noir-lang/acvm#151

@TomAFrench TomAFrench closed this Apr 4, 2023
@TomAFrench TomAFrench deleted the gd/issue_573 branch November 20, 2024 12:00
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.

Optimise away predicates which are always true

4 participants