Skip to content

Revert "Parallelise perm_aggreg sub arrays" #3105

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

Merged
merged 1 commit into from
Mar 27, 2025

Conversation

dannywillems
Copy link
Member

@dannywillems dannywillems commented Mar 26, 2025

This reverts commit 3647beb.

For the context, see MinaProtocol/mina#16782 (comment). The test in Mina is failing with the following error at this exact commit:

File "src/lib/pickles/test/chunked_circuits/dune", line 2, characters 8-15:
2 |  (names chunks2 chunks4 chunks8)
            ^^^^^^^
Uncaught exception:
  (monitor.ml.Error
   (Failure "Prover(\"rest of division by vanishing polynomial\")")
   ("Raised at Base__Result.ok_exn in file \"src/result.ml\", line 201, characters 17-26"
    "Called from Async_kernel__Deferred1.M.map.(fun) in file \"src/deferred1.ml\", line 17, characters 40-45"
    "Called from Async_kernel__Job_queue.run_job in file \"src/job_queue.ml\" (inlined), line 128, characters 2-5"
    "Called from Async_kernel__Job_queue.run_jobs in file \"src/job_queue.ml\", line 169, characters 6-47"
    "Caught by monitor block_on_async"))
Raised at Base__Result.ok_exn in file "src/result.ml" (inlined), line 201, characters 17-26
Called from Async_unix__Thread_safe.block_on_async_exn in file "src/thread_safe.ml", line 132, characters 29-63
Called from Async_unix__Thread_safe.block_on_async_exn in file "src/thread_safe.ml" (inlined), line 181, characters 27-54
Called from Dune__exe__Chunks2.test.test_prove in file "src/lib/pickles/test/chunked_circuits/chunks2.ml", line 116, characters 6-63
Called from Dune__exe__Chunks2 in file "src/lib/pickles/test/chunked_circuits/chunks2.ml", line 132, characters 2-9
File "src/lib/pickles/test/chunked_circuits/dune", line 2, characters 16-23:
2 |  (names chunks2 chunks4 chunks8)
                    ^^^^^^^
compile time: 0.100414s
Uncaught exception:
  (monitor.ml.Error
   (Failure "Prover(\"rest of division by vanishing polynomial\")")
   ("Raised at Base__Result.ok_exn in file \"src/result.ml\", line 201, characters 17-26"
    "Called from Async_kernel__Deferred1.M.map.(fun) in file \"src/deferred1.ml\", line 17, characters 40-45"
    "Called from Async_kernel__Job_queue.run_job in file \"src/job_queue.ml\" (inlined), line 128, characters 2-5"
    "Called from Async_kernel__Job_queue.run_jobs in file \"src/job_queue.ml\", line 169, characters 6-47"
    "Caught by monitor block_on_async"))
Raised at Base__Result.ok_exn in file "src/result.ml" (inlined), line 201, characters 17-26
Called from Async_unix__Thread_safe.block_on_async_exn in file "src/thread_safe.ml", line 132, characters 29-63
Called from Async_unix__Thread_safe.block_on_async_exn in file "src/thread_safe.ml" (inlined), line 181, characters 27-54
Called from Dune__exe__Chunks4.test.test_prove in file "src/lib/pickles/test/chunked_circuits/chunks4.ml", line 66, characters 6-63
Called from Dune__exe__Chunks4 in file "src/lib/pickles/test/chunked_circuits/chunks4.ml", line 77, characters 2-9
File "src/lib/pickles/test/chunked_circuits/dune", line 2, characters 24-31:
2 |  (names chunks2 chunks4 chunks8)
                            ^^^^^^^
compile time: 0.100318s
Uncaught exception:
  (monitor.ml.Error
   (Failure "Prover(\"rest of division by vanishing polynomial\")")
   ("Raised at Base__Result.ok_exn in file \"src/result.ml\", line 201, characters 17-26"
    "Called from Async_kernel__Deferred1.M.map.(fun) in file \"src/deferred1.ml\", line 17, characters 40-45"
    "Called from Async_kernel__Job_queue.run_job in file \"src/job_queue.ml\" (inlined), line 128, characters 2-5"
    "Called from Async_kernel__Job_queue.run_jobs in file \"src/job_queue.ml\", line 169, characters 6-47"
    "Caught by monitor block_on_async"))
Raised at Base__Result.ok_exn in file "src/result.ml" (inlined), line 201, characters 17-26
Called from Async_unix__Thread_safe.block_on_async_exn in file "src/thread_safe.ml", line 132, characters 29-63
Called from Async_unix__Thread_safe.block_on_async_exn in file "src/thread_safe.ml" (inlined), line 181, characters 27-54
Called from Dune__exe__Chunks8.test.test_prove in file "src/lib/pickles/test/chunked_circuits/chunks8.ml", line 66, characters 6-63
Called from Dune__exe__Chunks8 in file "src/lib/pickles/test/chunked_circuits/chunks8.ml", line 77, characters 2-9

@dannywillems dannywillems requested review from Copilot and volhovm and removed request for Copilot March 26, 2025 21:25
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request reverts the previous commit "Parallelise perm_aggreg sub arrays" by restoring sequential computation for permutation aggregation.

  • Replaces the parallel computation (using par_iter and reduce_with) with sequential loops.
  • Removes the z_prefolded vector computation and inlines the corresponding product accumulation.

@dannywillems dannywillems merged commit 103901a into master Mar 27, 2025
5 checks passed
@dannywillems dannywillems deleted the dw/revert-perm-aggreg-sub-arrays branch March 27, 2025 15:00
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