-
Notifications
You must be signed in to change notification settings - Fork 52
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
Implement comparisons in the circuit #143
Conversation
src/circuit/circuit_frame.rs
Outdated
|
||
let diff_is_negative_or_zero = constraints::or( | ||
&mut cs.namespace(|| "diff is negative or zero"), | ||
lsb_2diff.unwrap(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use diff_is_negative
here, since we (rightly) bothered creating this clarifying binding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To minimize cognitive load, I would get rid of these 'or_zero' names and standardize on a simpler convention. I would call this diff_is_not_positive
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, with a few comments about naming and/or logic that I think should be addressed. Once that's done, I think this is good to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, with a few hanging comments worth addressing before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
I restarted CI because the mac job timed out. That and the arm64 job have both become very slow. We should try to figure out why that is, and hopefully address it. That doesn't seem to be so on |
* Comparison of a and b Co-authored-by: porcuquine <porcuquine@users.noreply.github.com>
Add comparisons to the circuit. In order to do that I have refactored
multi_case
to returnis_default
, because it allows to avoid repeated constraints.