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.

Sign up for GitHub

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

Jump to bottom

Component testing framework assumes all constants are placed in a single column. #282

Open
dkales opened this issue Nov 15, 2023 · 1 comment · May be fixed by #284
Open

Component testing framework assumes all constants are placed in a single column. #282

dkales opened this issue Nov 15, 2023 · 1 comment · May be fixed by #284
Assignees
@Iluvmagick

Comments

@dkales
Copy link

dkales commented Nov 15, 2023

https://github.com/NilFoundation/zkllvm-blueprint/blob/93b6463d8d6378e08994c3208fbed34f90b30fa7/test/test_plonk_component.hpp#L286C38-L286C38

The component tester assumes that all components use at most 1 column for constants. We have components that in some configurations where the component fits into a single row, we require 2 constants, and therefore 2 constant columns.

Could the testing framework not get the number of used constant columns from the component itself?
Furthermore, the component stretcher also assumes that there is at most 1 constant column.
@Iluvmagick Iluvmagick self-assigned this Nov 16, 2023
@Iluvmagick
Copy link
Contributor

Sure, I can modify the framework so that it just uses all the "unassigned" constant columns for lookup table packing.

AFAIK currently the compiler is also assuming that all components use not more than a single constant column.

As for the stretcher: this is correct, but I am unsure if we are going to change this soon; we might rely more on manually implementing different variants of components, or consider different automation approaches.

@Iluvmagick Iluvmagick linked a pull request Nov 16, 2023 that will close this issue
Testing rig now uses all empty constant columns for lookup tables. #284
Open
@Iluvmagick Iluvmagick linked a pull request Nov 16, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Iluvmagick Iluvmagick

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Testing rig now uses all empty constant columns for lookup tables. NilFoundation/zkllvm-blueprint
2 participants
@dkales @Iluvmagick