Feature/optimise step selector builder #154

rutefig · 2023-10-16T08:29:41Z

Optimisation of Step Selector for Log2(n) Columns

Description

Goal: is to optimise the Step Selector Builder to have Log2(n) Columns per n Step Types instead of one Column per each Step Type.
Current Implementation: implemented a new type of Selector and implemented the build function for it
Explanation: If we use binary representation for the step types instead of using the actual columns we will get an optimised version for the Selector Builder since we can get the total of columns from the following expression

$$n\_cols = \lceil \log_2(n\_step\_types + 1) \rceil$$

We need take into consideration that we also need to save one binary value for the case when there is no step type, because of this we do the math with (n_step_types + 1).

What is missing

Ensure compatibility with the backend
Unit tests

…sion

…ons#151)

…explorations#111) Resolve privacy-scaling-explorations#85 - [x] TraceWitness - [x] Assignments --------- Co-authored-by: Leo Lara <leolara@users.noreply.github.com>

@leolara

Resolves privacy-scaling-explorations#129. @leolara @qwang98 Ready for review. Automatic instance is now enabled in plaf codebase,(Dhole/polyexen#3) and this PR would apply that change.

…-explorations#146) Now Expr arithmetic doesn't require LHS to be Expr but can be ToExpr as well. Thanks to Sandro for pointing out this error.

…before: StepSelector

… the number of cols

leolara · 2023-10-21T10:41:33Z

4 steps require 3 columns, because 00 is not used. Please add a test with 4 steps that checks that 3 columns are used.

…e have one value in use to indicate no step

src/plonkish/compiler/step_selector.rs

leolara · 2023-10-25T02:44:19Z

Rusfmt problems

leolara · 2023-10-26T11:34:56Z

I think you test the expresions and the assignments, the expressions can be converted to a string with print!(":#?", expr) and then compared to a string @rutefig

…-explorations#146) Now Expr arithmetic doesn't require LHS to be Expr but can be ToExpr as well. Thanks to Sandro for pointing out this error.

…types test

src/plonkish/compiler/step_selector.rs

codecov-commenter · 2023-10-30T10:28:39Z

Codecov Report

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

Comparison is base (009690b) 46.84% compared to head (8eaea22) 48.02%.
Report is 2 commits behind head on main.

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #154      +/-   ##
==========================================
+ Coverage   46.84%   48.02%   +1.17%     
==========================================
  Files          21       21              
  Lines        5364     5483     +119     
==========================================
+ Hits         2513     2633     +120     
+ Misses       2851     2850       -1

Files	Coverage Δ
src/plonkish/compiler/step_selector.rs	`49.21% <98.31%> (+43.28%)`	⬆️

... and 1 file with indirect coverage changes

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

rutefig · 2023-10-30T13:45:07Z

@leolara need approval here, the previous run failed on the Clippy so I committed a fix for that

rutefig and others added 11 commits October 13, 2023 12:07

implemented basic version of binary step builder with selector_expres…

b40dab4

…sion

added assignment to selector_expr_not and selector_assignment

21e758c

Table formatting in the Fibonacci Tutorial (privacy-scaling-explorati…

840dc53

…ons#151)

feat: add pretty debug information for common items (privacy-scaling-…

f9b3f84

…explorations#111) Resolve privacy-scaling-explorations#85 - [x] TraceWitness - [x] Assignments --------- Co-authored-by: Leo Lara <leolara@users.noreply.github.com>

Use plaf automatic instance (privacy-scaling-explorations#144)

5e0c616

Resolves privacy-scaling-explorations#129. @leolara @qwang98 Ready for review. Automatic instance is now enabled in plaf codebase,(Dhole/polyexen#3) and this PR would apply that change.

Fixed LHS auto typing conversion for Expr arithmetic (privacy-scaling…

b4d247e

…-explorations#146) Now Expr arithmetic doesn't require LHS to be Expr but can be ToExpr as well. Thanks to Sandro for pointing out this error.

LogN step builder now builds the selector with the same structure as …

991deeb

…before: StepSelector

fixed polynomial expressions for the optimised step builder

9f0b33a

fixed logic on build function and started to implement tests to check…

5bfe8d2

… the number of cols

implemented another test for 10 step types

19c7c8f

Merge branch 'main' into feature/optimise-step-selector-builder

1588a45

rutefig marked this pull request as ready for review October 20, 2023 04:25

rutefig added 3 commits October 23, 2023 17:34

removed print

60df017

fixed calculation for number of cols - we need to add 1 extra since w…

85c8d56

…e have one value in use to indicate no step

Merge branch 'main' into feature/optimise-step-selector-builder

b518553

leolara reviewed Oct 24, 2023

View reviewed changes

src/plonkish/compiler/step_selector.rs Outdated Show resolved Hide resolved

leolara reviewed Oct 24, 2023

View reviewed changes

src/plonkish/compiler/step_selector.rs Outdated Show resolved Hide resolved

rutefig added 3 commits October 26, 2023 10:40

refactored tests

2c82777

bug fixes, code refactor and added more assertions to tests

e40458b

removed useless comments

c16b8a6

rutefig requested a review from leolara October 26, 2023 06:52

Merge branch 'main' into feature/optimise-step-selector-builder

894deb8

qwang98 and others added 5 commits October 26, 2023 20:54

Fixed LHS auto typing conversion for Expr arithmetic (privacy-scaling…

ae1a796

…-explorations#146) Now Expr arithmetic doesn't require LHS to be Expr but can be ToExpr as well. Thanks to Sandro for pointing out this error.

fixed formatting

2960668

Merge branch 'main' into feature/optimise-step-selector-builder

9b0746a

assert string representation for the selector expressions for 3 step …

af665b6

…types test

Merge branch 'main' into feature/optimise-step-selector-builder

5642dc6

leolara reviewed Oct 30, 2023

View reviewed changes

src/plonkish/compiler/step_selector.rs Show resolved Hide resolved

updated expression not to be calculated based on the expression

de44409

leolara approved these changes Oct 30, 2023

View reviewed changes

fix clippy check

b0a42ea

leolara approved these changes Oct 30, 2023

View reviewed changes

leolara added this pull request to the merge queue Oct 30, 2023

Merged via the queue into privacy-scaling-explorations:main with commit 93e0fad Oct 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/optimise step selector builder #154

Feature/optimise step selector builder #154

rutefig commented Oct 16, 2023 •

edited

Loading

leolara commented Oct 21, 2023

leolara commented Oct 25, 2023

leolara commented Oct 26, 2023

codecov-commenter commented Oct 30, 2023 •

edited

Loading

rutefig commented Oct 30, 2023

Feature/optimise step selector builder #154

Feature/optimise step selector builder #154

Conversation

rutefig commented Oct 16, 2023 • edited Loading

Optimisation of Step Selector for Log2(n) Columns

Description

What is missing

leolara commented Oct 21, 2023

leolara commented Oct 25, 2023

leolara commented Oct 26, 2023

codecov-commenter commented Oct 30, 2023 • edited Loading

Codecov Report

rutefig commented Oct 30, 2023

rutefig commented Oct 16, 2023 •

edited

Loading

codecov-commenter commented Oct 30, 2023 •

edited

Loading