Skip to content

Conversation

@Fantoni0
Copy link
Collaborator

This PR updates the compute_h and compute_h2 functions in the pointvsline.rs file.
These functions iteratively fold multiple polynomials until a single one is computed as result.
The original implementation used DensePolynomial for the inner computations, which is fine. But for large MLEs this causes excessive allocations and intermediate objects that are soon discarded.

This PR makes 2 significant changes:

  • Adds parallel iterators.
  • Instead of using the DensePolynomial struct, we work over the coefficients of the polynomials directly.

This approach seems to be fine, I have compared the results against the original implementation (you can do this by replacing the compute_h function with the original implementation). I have also extended the tests cases for the test_compute_h2_compare test function.

Results:

  • Parallel iterators do not help, might even slighltly worsen the results, for small MLEs. But I believe all our applications leverage large polynomials.
  • Similarly, the optimizations do not seem to be significan for small MLEs. But for large cases, and successive foldings, it gets x2 or even x10 faster. You can run cargo bench --bench mova_matrix to test it. I tried size 16 and 512.

@Fantoni0 Fantoni0 requested a review from NiDimi June 12, 2025 08:29
Copy link
Collaborator

@NiDimi NiDimi left a comment

Choose a reason for hiding this comment

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

The changes look good. Tested the benchmarks and there is a noticable increase in perforamance when we fold multiple times. Great work Antonio 🔥. Can you add in the benches description the size of the matrices? Then we can merge it.

@Fantoni0 Fantoni0 merged commit 8bb79c9 into main_folding Jun 26, 2025
6 checks passed
@Fantoni0 Fantoni0 deleted the feat/update_compute_h branch June 26, 2025 11:17
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.

3 participants