Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

Automatic signals generation #152

Merged
merged 4 commits into from
Nov 1, 2023
Merged

Automatic signals generation #152

merged 4 commits into from
Nov 1, 2023

Conversation

leolara
Copy link
Collaborator

@leolara leolara commented Oct 15, 2023

This is useful for:

  • Arbitrary boolean expressions
  • Multiplicative inverse elimination
  • Automatic witness generation in general
  • Conversion to lower degree polynomial identities

@leolara leolara requested a review from qwang98 October 15, 2023 16:31
src/wit_gen.rs Outdated Show resolved Hide resolved
src/wit_gen.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@qwang98 qwang98 left a comment

Choose a reason for hiding this comment

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

Overall, this PR allows the auto_eq feature which automatically assigns the left hand side queriable of a regular constraint. I'm good with the implementation of this feature and left a couple comments on some potential improvements. Let me know if you agree! @leolara

Looking forward, there are more things we can do:

  1. Allow auto witness generation for constraints other than eq.
  2. Allow auto witness generation for transition constraints and signals with rotation as well.
  3. Similar to PIL, we could eventually enable an auto witness generation "mode" where instead of using the auto_eq API, the user only needs to provide the initial trace input and all signals in all step instances will be inferred, or return an error saying that not enough trace input was provided.

@qwang98 qwang98 self-requested a review October 30, 2023 16:14
Copy link
Collaborator

@qwang98 qwang98 left a comment

Choose a reason for hiding this comment

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

LGTM with additional features to be implemented in future PRs.

@qwang98
Copy link
Collaborator

qwang98 commented Oct 30, 2023

@leolara Good to go after fixing the conflicts.

@leolara leolara added this pull request to the merge queue Nov 1, 2023
@leolara leolara removed this pull request from the merge queue due to a manual request Nov 1, 2023
@codecov-commenter
Copy link

Codecov Report

Attention: 35 lines in your changes are missing coverage. Please review.

Comparison is base (93e0fad) 48.02% compared to head (bb0e74b) 49.81%.

❗ Current head bb0e74b differs from pull request most recent head eb35cca. Consider uploading reports for the commit eb35cca to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #152      +/-   ##
==========================================
+ Coverage   48.02%   49.81%   +1.79%     
==========================================
  Files          21       21              
  Lines        5483     5729     +246     
==========================================
+ Hits         2633     2854     +221     
- Misses       2850     2875      +25     
Files Coverage Δ
src/ast/mod.rs 41.41% <100.00%> (+0.18%) ⬆️
src/plonkish/ir/sc.rs 0.00% <ø> (ø)
src/plonkish/compiler/mod.rs 0.00% <0.00%> (ø)
src/poly.rs 86.30% <96.49%> (+12.14%) ⬆️
src/plonkish/backend/halo2.rs 0.00% <0.00%> (ø)
src/plonkish/ir/assignments.rs 16.08% <0.00%> (-0.50%) ⬇️
src/frontend/dsl/mod.rs 16.86% <0.00%> (-0.56%) ⬇️
src/wit_gen.rs 87.46% <90.58%> (+4.59%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@leolara leolara added this pull request to the merge queue Nov 1, 2023
Merged via the queue into main with commit 726b2a9 Nov 1, 2023
4 checks passed
@alxkzmn alxkzmn deleted the leo/auto_signals branch August 7, 2024 11:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants