Skip to content
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

🙈 change default minmax_ratio to 4 #62

Merged
merged 2 commits into from
Nov 24, 2023
Merged

🙈 change default minmax_ratio to 4 #62

merged 2 commits into from
Nov 24, 2023

Conversation

jvdd
Copy link
Member

@jvdd jvdd commented Nov 23, 2023

Changes the minmax_ratio to 4 (was previously 30) - this change is based on the empirical evidence gathered in our research on the MinMaxLTTB work; https://arxiv.org/abs/2305.00332 (in this paper minmax_ratio is called the "preselection ratio").

Note that in plotly-resasampler we already used 4 as default value (in the wrapper for this package);
https://github.com/predict-idlab/plotly-resampler/blob/741da5d2358bee59c59d1591b0722ab0ea66bbb6/plotly_resampler/aggregation/aggregators.py#L211

This PR also fixes a minor bug when calling MinMaxLTTB.downsample (without an x array) -> the indices of the preselected data points should be passed to the LTTB algorithm (2nd step of the algorithm) in order to have a more correct triangle surface calculation (-> will resemble more closesly vanilla LTTB behavior).


Credits are due to @raphaelvallat for uncovering this issue

Copy link

codspeed-hq bot commented Nov 23, 2023

CodSpeed Performance Report

Merging #62 will degrade performances by 92.12%

Comparing minmax_ratio (aa937c5) with main (25d5648)

Summary

⚡ 174 improvements
❌ 23 regressions
✅ 193 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main minmax_ratio Change
test_minmax_no_x[False-float32-100-100,000] 8.1 ms 1.6 ms ×5.1
test_minmax_no_x[False-float32-100-1,000,000] 69.4 ms 4.6 ms ×15
test_minmax_no_x[False-float32-5,000-100,000] 9.3 ms 2.2 ms ×4.2
test_minmax_no_x[False-float32-5,000-1,000,000] 71.5 ms 6.1 ms ×12
test_minmax_no_x[False-float64-100-1,000,000] 139.8 ms 9 ms ×16
test_minmax_no_x[False-float64-100-100,000] 15 ms 1.9 ms ×7.8
test_minmax_no_x[False-float64-5,000-100,000] 16.2 ms 2.6 ms ×6.2
test_minmax_no_x[False-float64-1,000-100,000] 14.8 ms 1.6 ms ×9.3
test_minmax_no_x[False-float64-5,000-1,000,000] 141.6 ms 10.2 ms ×14
test_minmax_no_x[True-float32-100-1,000,000] 69.6 ms 5 ms ×14
test_minmax_no_x[True-float32-100-100,000] 7.6 ms 1.3 ms ×6
test_minmax_no_x[True-float32-1,000-1,000,000] 70.1 ms 5.6 ms ×13
test_minmax_no_x[False-float32-1,000-1,000,000] 69.8 ms 4.9 ms ×14
test_minmax_no_x[True-float32-5,000-100,000] 9.8 ms 2.9 ms ×3.4
test_minmax_no_x[True-float64-100-100,000] 14.7 ms 1.8 ms ×8.1
test_minmax_no_x[True-float64-100-1,000,000] 140.1 ms 9.6 ms ×15
test_minmax_no_x[False-float32-1,000-100,000] 7.7 ms 1.1 ms ×7.1
test_minmax_no_x[True-float64-5,000-1,000,000] 142.1 ms 10.8 ms ×13
test_minmax_no_x[True-int32-1,000-100,000] 2 ms 2.3 ms -14.02%
test_minmax_no_x[True-float64-5,000-100,000] 16.6 ms 3.3 ms ×5
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@jvdd jvdd merged commit 18b9fdf into main Nov 24, 2023
37 of 38 checks passed
@jvdd jvdd deleted the minmax_ratio branch January 3, 2024 12:10
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.

1 participant