Closed hhirtz closed 2 years ago
Base: 51.64% // Head: 51.75% // Increases project coverage by +0.10%
:tada:
Coverage data is based on head (
0df0225
) compared to base (59882a9
). Patch coverage: 30.00% of modified lines in pull request are covered.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Previously, weighted_quantiles would modify "splits" in place. While no actual bug or imbalance violations have been observed, this caused "splits" and "part_weights" to go out of sync. Later iterations of the inner for loop would use the newer splits with the former values of part_weights.
The split update is not done in parallel, because the overhead of rayon becomes too costly with high core counts.
It seems that this version also scales a bit better than master's, maybe because the for loop has been replaced with a closure and leads to better behavior wrt. the instruction cache? We'll never know.