Skip to content

perf: Don't do fused-multiply-add on scalars#27479

Merged
ritchie46 merged 3 commits intopola-rs:mainfrom
orlp:no-scalar-fma
May 4, 2026
Merged

perf: Don't do fused-multiply-add on scalars#27479
ritchie46 merged 3 commits intopola-rs:mainfrom
orlp:no-scalar-fma

Conversation

@orlp
Copy link
Copy Markdown
Member

@orlp orlp commented May 1, 2026

If both the inputs to the multiply are scalars it doesn't make much sense to do the optimization. I also refactored the optimization code a bit to be cleaner, and made the FMA code work on scalar columns directly rather than materializing to series.

I also spotted a pretty bad bug where FMA wasn't a * b + c but rather a + b * c, but elsewhere in the code we assumed it was a * b + c: https://github.com/orlp/polars/blob/no-scalar-fma/crates/polars-plan/src/plans/conversion/ir_to_dsl.rs#L1021. So I've made it consistent with the typical a * b + c.

@github-actions github-actions Bot added performance Performance issues or improvements python Related to Python Polars rust Related to Rust Polars labels May 1, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 1, 2026

Codecov Report

❌ Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.63%. Comparing base (62674c4) to head (4cc783d).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
crates/polars-ops/src/series/ops/fused.rs 50.00% 2 Missing ⚠️
crates/polars-plan/src/plans/optimizer/fused.rs 96.42% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #27479      +/-   ##
==========================================
+ Coverage   81.34%   81.63%   +0.29%     
==========================================
  Files        1838     1840       +2     
  Lines      254455   254672     +217     
  Branches     3180     3180              
==========================================
+ Hits       206978   207894     +916     
+ Misses      46653    45954     -699     
  Partials      824      824              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ritchie46
Copy link
Copy Markdown
Member

Could this bug be triggered? I would expect this to be hit. Can we add a test.

Comment thread crates/polars-plan/src/plans/optimizer/fused.rs
@ritchie46 ritchie46 merged commit 136214c into pola-rs:main May 4, 2026
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance issues or improvements python Related to Python Polars rust Related to Rust Polars

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants