You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As already discussed with @therealyingtong on Discord, we think that the query_instance() method should take an absolute row: usize instead of a relative Rotation as input parameters. Instance values do not interact with regions / the layouter.
This would also make query_instance() consistent with assign_advice_from_instance(), which already requires the caller to provide the absolute row of the specified instance column.
rhs is a public input passed in the instance column at row 3. Right now, in the assignment function, the rhs value is copied from the instance column to the advice column.
pub fn enforce_less_than {
layouter.assign_region(
|| "enforce sum to be less than total assets",
|mut region| {
...
// copy the total assets from instance column at row 3
region.assign_advice_from_instance(
|| "copy total assets",
self.config.instance,
3,
self.config.advice[1],
0
)?;
...
},
)?;
}
A more convenient way would be configuring the chip by passing directly an expression from the instance column. This will remove the need for an extra copy constraint
Why is there a query_instance method at all? With the other query_* methods, the aim is to be able to construct a gate polynomial that references the given column at the specified offset. These references are necessarily relative, because that's just how gate polynomials work. But unless you have complete control of layout (which would potentially allow you to arrange gates on absolute rows next to the relevant instance cells), that doesn't work for instance columns. So the only practical way to reference an instance cell in a way that works with the layouter abstraction is to use an equality constraint, which is what assign_advice_from_instance does.
We might want to retain a way for a circuit to use absolute layout, but that shouldn't be mixed in with the layouter-based API, I think. In any case I don't think that retaining query_instance but making the second argument an absolute row number is the right approach. (If we did do this, it's incompatible with any existing correct use of query_instance and inconsistent with the other query_* methods, so we should at least change the method name.)
I agree that the current query_instance method is confusing and difficult to use.
Every idea that I have of adding an API to support such a feature adds unnecessary complexity at the (small) benefit of saving a copy constraint. I will close the issue unless you, or any other member of the community, decide it is worth it.
Also, back to the previous scenario, querying the instance column at absolute row 3 could create a huge region since the cell in the instance column might be very "far" from the cells in the advice columns. Would that increase the prover cost?
As already discussed with @therealyingtong on Discord, we think that the
query_instance()
method should take an absoluterow: usize
instead of a relativeRotation
as input parameters. Instance values do not interact with regions / the layouter.This would also make
query_instance()
consistent withassign_advice_from_instance()
, which already requires the caller to provide the absolute row of the specified instance column.An example where this would be required:
This is the current configuration of a chip
rhs
is a public input passed in the instance column at row 3. Right now, in the assignment function, the rhs value is copied from the instance column to the advice column.A more convenient way would be configuring the chip by passing directly an expression from the instance column. This will remove the need for an extra copy constraint
In this case, it is unclear how to use the
Rotation
to query the instance column at a specific row. A more intuitive API would beThe text was updated successfully, but these errors were encountered: