Closed-form RF calculation for read-only-kv#216
Closed
homatthew wants to merge 3 commits intoNetflix-Skunkworks:chengw/oodmfrom
Closed
Closed-form RF calculation for read-only-kv#216homatthew wants to merge 3 commits intoNetflix-Skunkworks:chengw/oodmfrom
homatthew wants to merge 3 commits intoNetflix-Skunkworks:chengw/oodmfrom
Conversation
2196dcc to
d355c43
Compare
- Same algorithm, same results, cleaner code - O(1) instead of O(rf) iterations - All 29 existing tests pass (including Hypothesis property tests) Co-Authored-By: Claude Opus 4.5 <[email protected]>
d355c43 to
29797a3
Compare
- Add _PartitionSearchInputs dataclass to represent pure numeric inputs - Extract _extract_planning_inputs() to handle model→algorithm transformation - Simplify _compute_read_only_kv_regional_cluster() to 3 clear steps: 1. Extract inputs, 2. Run algorithm, 3. Build result - Make _find_valid_cluster_config use keyword-only args (pylint fix) - Improves testability and documents data flow explicitly Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ations - Create partition_capacity.py with three algorithm variants: - original_algorithm: while-loop (greedy, max PPn only) - closed_form_algorithm: O(1) mathematical equivalent - search_algorithm: searches PPn from max to 1, finds solutions original misses - Add test_partition_capacity.py demonstrating: - CLOSED_FORM == ORIGINAL (always, by mathematical proof) - SEARCH ⊇ ORIGINAL (search finds everything original finds) - SEARCH ≠ ORIGINAL when max_nodes is tight (search finds solutions original misses) - Update read_only_kv.py to use search_algorithm from new module - Update test_read_only_kv.py to import from partition_capacity module The extracted module makes it easy to understand the algorithm without following private methods, and the parametrized tests clearly show where the algorithms are equivalent vs. different. Co-Authored-By: Claude Opus 4.5 <[email protected]>
This file contains hidden or 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
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.
What am I trying to do?
Improve the partition-aware capacity planning algorithm in
read_only_kv.py:partition_capacity.pymodule for clarityWhy did I do it this way?
Standalone
partition_capacity.pymodule (new!)The algorithm logic was buried in private methods inside
read_only_kv.py. This made it hard to:Solution: Extract to
partition_capacity.pywith three clear algorithm variants:Three algorithm variants with clear relationships
original_algorithmclosed_form_algorithmsearch_algorithmKey findings documented in tests:
Closed-form RF calculation (commit 1)
The original
while Trueloop incremented RF until CPU was satisfied. This obscures the underlying math.The insight: For a given PPn, we can directly compute the minimum RF:
Extract
_PartitionSearchInputsdataclass (commit 2)Clear boundary between "model domain" (Instance, CapacityRequirement) and "algorithm domain" (pure numbers).
Are there any tests?
Yes - 79 tests total including:
How would I use the new code?
The public interface (
nflx_read_only_kv_capacity_model.capacity_plan()) is unchanged.For direct algorithm testing:
Architecture
graph TB subgraph partition_capacity.py A[CapacityProblem] --> B[original_algorithm] A --> C[closed_form_algorithm] A --> D[search_algorithm] B --> E[CapacityResult] C --> E D --> E end subgraph read_only_kv.py F[Model Objects] --> G[_extract_planning_inputs] G --> H[_PartitionSearchInputs] H --> I[CapacityProblem] I --> D E --> J[RegionClusterCapacity] end🤖 Generated with Claude Code