-
Notifications
You must be signed in to change notification settings - Fork 550
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
Update arkworks to 0.4.2 (compatible) #15940
Conversation
volhovm
commented
Aug 19, 2024
•
edited
Loading
edited
- Proof-systems: Upgrade to arkworks 0.4.2 (mina compatible / p-s develop) o1-labs/proof-systems#2474
- Kimchi-stubs vendors: Update arkworks to 0.4.2 (mina compatible) kimchi-stubs-vendors#1
a17d527
to
dac2318
Compare
So with a small fix to
|
1a23041
to
d19a8d5
Compare
d19a8d5
to
74e1863
Compare
After several tweaks (and the last bugfix addressing
And here's a second run, one minute faster than another. A lot of variance, but there seems to be no regression anymore:
|
Here's a summary of more runs on my machine. Seems that there is no regression anymore, the difference is most likely within error range potentially caused by uneven system load.
Data from runs: Compatible runs (
Arkworks runs:
|
74e1863
to
d5b3e13
Compare
d5b3e13
to
f8e2e1a
Compare
f8e2e1a
to
c698f33
Compare
!ci-build-me |
Testing running the same command than here. |
This branch:
On compatible (34af10d), I'm getting a segfault atm:
|
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.
See above. LGTM reg benchmark otherwise.
Re segfault: I also had this during the first time I was trying to reproduce the build, but then it disappeared.
|
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.
Sorry, I did not want to approve, yet.
Smart, smart... Forgot to recompile mina.exe... 🫤 |
Thanks @dannywillems! Merging now cc @georgeee @mrmr1993. In summary:
That is I think it's a reasonable evidence we're still backward compatible for now. |