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

Make MaxWidthCellManager work with fixed signals #97

Open
leolara opened this issue Aug 23, 2023 · 6 comments · May be fixed by #135
Open

Make MaxWidthCellManager work with fixed signals #97

leolara opened this issue Aug 23, 2023 · 6 comments · May be fixed by #135
Assignees
Labels
gfi intermediate Intermediate good first issue good first issue Good for newcomers

Comments

@leolara
Copy link
Collaborator

leolara commented Aug 23, 2023

Currently only SingleRowCellManager can handle fixed signals. Make it work on MaxWidthCellManager.

Because the fixed signals use different columns than the other signals, we need a new parameter called max_width_fixed.

@atsupontio
Copy link

I would like to try working on this issue!

@leolara
Copy link
Collaborator Author

leolara commented Sep 16, 2023

@atsupontio assigned

atsupontio added a commit to atsupontio/chiquito that referenced this issue Sep 18, 2023
@atsupontio
Copy link

I dont know where a new parameter max_width_fixed should be added, should it be added to the MaxWidthCellManager structure and used separately from max_width?

Also, I have an implementation in my branch without max_width_fixed added.
I ran poseidon.rs with this implementation as let single_config = config(MaxWidthCellManager::new(2, true), SimpleStepSelectorBuilder {}); #78 and it worked fine.

Is there anything I should fix?

@leolara
Copy link
Collaborator Author

leolara commented Sep 18, 2023

MaxWidthCellManager should have a new field called max_width_fixed, also rename max_width to max_width_advice.

https://github.com/privacy-scaling-explorations/chiquito/blob/main/src/plonkish/compiler/cell_manager.rs#L233

atsupontio added a commit to atsupontio/chiquito that referenced this issue Sep 19, 2023
@atsupontio
Copy link

I added max_width_fixed and renamed max_width to max_width_advice.
There is implementation in my branch, so please take a look!
And, please let me know if anything else is required.

@leolara
Copy link
Collaborator Author

leolara commented Sep 19, 2023

@atsupontio for us to review code has to be sent on a PR, it can be draft mode if you are unsure.

atsupontio added a commit to atsupontio/chiquito that referenced this issue Sep 19, 2023
atsupontio added a commit to atsupontio/chiquito that referenced this issue Sep 20, 2023
atsupontio added a commit to atsupontio/chiquito that referenced this issue Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gfi intermediate Intermediate good first issue good first issue Good for newcomers
Projects
Status: In review
Development

Successfully merging a pull request may close this issue.

2 participants