Skip to content

perf: Various improvements to chisquare#336

Merged
YeungOnion merged 3 commits intostatrs-dev:masterfrom
FreezyLemon:optimize-chisquare-test
Jul 25, 2025
Merged

perf: Various improvements to chisquare#336
YeungOnion merged 3 commits intostatrs-dev:masterfrom
FreezyLemon:optimize-chisquare-test

Conversation

@FreezyLemon
Copy link
Contributor

@FreezyLemon FreezyLemon commented Apr 9, 2025

The current code always allocates a new f_exp: Vec<f64> just to iterate through it later. This is not necessary:

  • if f_exp is Some(..), we can iterate through the slice itself
  • if f_exp is None, we don't need to iterate anything. we can just set a fixed value for e and use that.

This has the added benefit of being no_std-compatible.

The amount of iterations through f_obs can also be reduced in some cases. If we need to f_obs.iter().zip(f_exp) anyways, there's no reason to do f_obs.iter().sum() beforehand. We can just calculate the sum "on the way", so to speak. The same goes for f_exp.

EDIT: I used approx::relative_eq since nothing comparable exists in crate::prec yet (on master). When #325 is merged, this should be updated to use the prec equivalent of relative_eq.

- avoid allocations (-> no_std)
- reduce number of .iter()
@codecov
Copy link

codecov bot commented Apr 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.99%. Comparing base (526e85e) to head (0af8efb).
Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #336      +/-   ##
==========================================
- Coverage   95.01%   94.99%   -0.02%     
==========================================
  Files          60       61       +1     
  Lines       13529    13615      +86     
==========================================
+ Hits        12854    12934      +80     
- Misses        675      681       +6     

☔ 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.

@YeungOnion
Copy link
Contributor

whoops! used the web editor for the merge and missed the closing brace. Will fix and retry.

@YeungOnion
Copy link
Contributor

oh man, I boofed it. I think I'll need you to fix it. Missing semicolon closing src/stats_tests/chisquare.rs:90

@FreezyLemon
Copy link
Contributor Author

No worries

@YeungOnion YeungOnion merged commit 013bb58 into statrs-dev:master Jul 25, 2025
7 of 8 checks passed
@FreezyLemon FreezyLemon deleted the optimize-chisquare-test branch July 26, 2025 08:20
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