str4d
commented
1 month ago Thanks for the review! @daira, @mmaller and I looked over it today. Comments below are from all of us.
Plonkish Relation Section:
- I found it quite challenging to read the table on instances (especially the last lines) at the beginning of the document because it introduces numerous notations that are explained later in the document.
Do you mean this table? https://github.com/zkpstandard/wg-plonkish/blob/7f8e78a95bf5edc228069d5df73abd40e88a2a5a/src/relation.md?plain=1#L35-L55
The intention is that this table is defining variables, that are specified later in the document. All notation should however have already been either defined or imported by reference here, in the Dependencies and notation section.
From a scan through the table, this is what we currently think might be notation that is not defined at this point:
- $\mathrel{⦂}$ (used to specify types)
- $[0,m) \times [0,n)$ (a product or tuple type)
- $\mathbb{F}^{m \times n}$ (a sequence type)
- $a \mapsto b$ (a mapping that is part of a function or relation)
- $A \rightarrow B$ (a function type) (this will replace the uses of $\mapsto$ in the relation document, which were incorrect)
- Multivariate polynomial
Is there any specific notation (of the above or otherwise) that you particularly found unclear?
- To improve the document's readability, I suggest to add simple examples: either a global example that includes each type of constraint or one example per constraint. If it's a global example, it might be beneficial to place it at the beginning of the document (before instance section) to start introducing some notations early on.
Part of the issue here is that because the overall documentation is not yet being rendered (because the repo was private for a while), it isn't obvious that the individual pages are not standalone. Once #30 is complete, the rendered version will make clear that relation.md and optimizations.md are specifications within the overall documentation, and we will consider adding a separate section with examples.
Backend Optimization Section:
- The purpose of transitioning from an abstract circuit to a concrete circuit is, in my opinion, not clear enough. I wondered if the goal was to optimize the number of rows, the number of columns, or both.
We had rewritten the background in #28 to say this, which I think addresses the comment:
The benefit of the simpler abstract model is that it allows reasoning about the circuit without needing to be concerned with layout of the witness matrix, i.e. the position of rows relative to each other. The drawback of using only the abstract model, is that it may require a greater number of columns to express a given circuit than a model that supports offsets. This directly affects proof size for popular instantiations of proof systems based on Plonkish.
We've now merged this PR; if you have further feedback on the current language, please let us know.
- I have difficulty understanding the last algorithm (algorithm for choosing $r$). Why don't the hints $h_i$ appear in it? How can we verify $\mathsf{ok＿for}(...)$ if the hints $h_i$ are not defined?
$\mathsf{ok＿for}(...)$ implicitly closes over both $h_i$ and $e_i$. In particular, if two abstract cells are both constrained, they can't map to the same concrete cell. And the definition of which concrete cell they map to is dependent on the hints.
Would this be clearer if $\mathsf{ok＿for}(...)$ took $\big[ (h_i, e_i) \big]_i$ as an explicit argument? [Edit: that has been completed.]
Finally, I noticed an inaccuracy in the definition of a multivariate polynomial. I believe that $c_z$ and $\nu$ should not be equal to 0.
You are right about $c_z$. [Edit: that has also been addressed.] Any multivariate polynomial with a $c_z = 0$ term can be rewritten to an equivalent multivariate polynomial with no $cz = 0$ terms and a smaller $\nu$. However, in order for that to allow expressing the zero polynomial, we must allow $\nu$ to be zero. (We use the common convention that ${\large \Sigma}{z = 0}^{\nu - 1} (\ldots) = 0$ when $\nu = 0$.)
ConstanceBeguier
Hello,
I have some feedback on your technical document.
Plonkish Relation Section:
Backend Optimization Section:
Finally, I noticed an inaccuracy in the definition of a multivariate polynomial. I believe that c_z and nu should not be equal to 0.