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

Upgrade icicle library to v3.4.0 #1172

Merged
merged 1 commit into from
Jan 29, 2025
Merged

Conversation

dmanc
Copy link
Contributor

@dmanc dmanc commented Jan 28, 2025

Why are these changes needed?

Upgrades icicle library to latests version. Didn't notice much improvement on GPU side but the CPU icicle backend made some improvements. Specifically seems their FFT is faster than the one we're using.

CPU comparison:
image
image
image

Checks

  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@dmanc dmanc requested review from bxue-l2 and jianoaix January 28, 2025 23:32
@dmanc dmanc marked this pull request as ready for review January 28, 2025 23:32
@jianoaix
Copy link
Contributor

So the existing Gnark is significantly faster than icicle on CPUs overall?

@dmanc
Copy link
Contributor Author

dmanc commented Jan 29, 2025

So the existing Gnark is significantly faster than icicle on CPUs overall?

Only for MSM, the FFT is much faster on icicle 3.4.0. But overall there is still a gap due to the MSM performance but it's closing in.

@dmanc dmanc merged commit 3a76e19 into Layr-Labs:master Jan 29, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants