Feature: Customize spread to maintain bigger emptiness in the rank ends. #195
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Frysch <anders.fryksborn@gmail.com> Co-authored-by: Caleb Buxton <caleb.buxton@babylist.com>
brendon
requested changes
Feb 6, 2023
| modifier = min < max ? 1 : -1 | ||
|
|
||
| if (max - min).abs < ranker.preferred_spread * 2 | ||
| rank_at(min + (max - min).abs * 0.3 * modifier) |
There was a problem hiding this comment.
Can you walk me through the 0.3? :)
| @@ -229,11 +252,13 @@ def rebalance_ranks | |||
| gaps = current_order.size + 1 | |||
| range = (RankedModel::MAX_RANK_VALUE - RankedModel::MIN_RANK_VALUE).to_f | |||
| gap_size = (range / gaps).ceil | |||
| gap_size = ranker.preferred_spread if ranker.preferred_spread && gap_size > ranker.preferred_spread | |||
| base_position = ranker.preferred_spread ? -gap_size * ((current_order.count + 1) / 2).ceil : RankedModel::MIN_RANK_VALUE | |||
There was a problem hiding this comment.
Also a quick explainer of this would be helpful. I'm in a bit of a rush so it'd save me some figuring out time :D
| @@ -170,12 +179,26 @@ def update_index_from_position | |||
| end | |||
| end | |||
|
|
|||
| def possibly_rebalance_and_position(min, max) | |||
There was a problem hiding this comment.
I think this could be improved in terms of how it's used and reads :) I'll have a think.
|
@cpb, do you still want to progress this? I'll close it for now, but let me know if you do and I'll re-open it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Co-authored-by: Frysch anders.fryksborn@gmail.com
Co-authored-by: Caleb Buxton caleb.buxton@babylist.com
Rebases the preferred_spread Feature atop of the latest:
Requesting Clarification:
I've added test coverage that illustrates rebalancing during adds.
This is an opt-in feature, so there isn't impact on the cases the original implementation is optimized for … which is to say: very few additions, and quite a lot of reordering.
While the maximum integer available in MySQL is a rather large number, its
log2isn't. The strategy provided by @frysch supports flexible allocation of the MySQL maximum integer space, so users can tune it to their use case.