-
Notifications
You must be signed in to change notification settings - Fork 2
feat(algorithms, binary-search): max runtime for n computers #110
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
Conversation
WalkthroughThis PR introduces a complete binary search solution for determining the maximum runtime for N computers given a set of batteries. It includes documentation describing the problem and approach, a Python implementation with multiple algorithm variants, and parameterized unit tests validating the solutions against test cases. Changes
Sequence DiagramsequenceDiagram
participant Client
participant max_runtime as max_runtime()
participant Binary Search
participant can_run_for as can_run_for()
Client->>max_runtime: batteries, n
activate max_runtime
max_runtime->>Binary Search: left=0, right=sum(batteries)//n
loop while left < right
Binary Search->>Binary Search: mid = left + (right-left)//2
Binary Search->>can_run_for: batteries, n, mid
activate can_run_for
can_run_for->>can_run_for: sum(min(battery, mid)) >= n*mid?
alt Feasible (true)
can_run_for-->>Binary Search: true
Binary Search->>Binary Search: left = mid + 1
else Not Feasible (false)
can_run_for-->>Binary Search: false
Binary Search->>Binary Search: right = mid
end
deactivate can_run_for
end
Binary Search-->>max_runtime: convergence
max_runtime-->>Client: max_runtime value
deactivate max_runtime
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
DIRECTORY.md (1)
96-97: New binary search entry looks consistent; consider md indentation to satisfy MD007.The added “Maxruntime N Computers” entry and test link are aligned with existing structure and point to the correct test path. Markdownlint (MD007, ul-indent) is complaining about the indentation on these bullets, though, so if your CI enforces that rule you may want to tweak the leading spaces (or adjust the rule) to keep the linter green.
algorithms/search/binary_search/maxruntime_n_computers/README.md (1)
31-68: Tighten up typos and notation in the README.The narrative matches the implemented algorithms, but there are a few small doc issues worth fixing:
- Line 43:
rught→rightin the midpoint formula.- Line 46: “the target feasible” → “the target is feasible” (or similar).
- Lines 62–64: clean up the extra closing parentheses in
O(n⋅logT)and inT = sum(batteries) // n), and be consistent about whatndenotes (number of computers vs number of batteries).- Line 68:
os→is, and consider ending with a period.These fixes will make the explanation crisper and avoid confusion for readers following the math.
algorithms/search/binary_search/maxruntime_n_computers/test_max_runtime.py (1)
1-38: Solid parameterized coverage for both implementations; minor DRY opportunity.The test cases and expected values look correct for the problem definition, and validating both
max_runtimeandmax_run_time_2against the same scenarios is a nice consistency check.If you want to reduce duplication, you could extract the shared case list to a module-level constant and reuse it in both
@parameterized.expand(...)decorators.algorithms/search/binary_search/maxruntime_n_computers/__init__.py (1)
4-73: Binary search + feasibility check implementation looks correct; small reuse/clarity tweaks optional.Both
max_runtimeandmax_run_time_2correctly:
- Bound the search by
sum(batteries) // n.- Use the monotone feasibility check
sum(min(b, t)) >= n * t.- Handle the 0-minute baseline and converge to the maximum feasible runtime without off‑by‑one issues.
A couple of optional refinements you might consider:
- Reuse
can_run_forinsidemax_run_time_2instead of duplicating theusable >= mid * nlogic to keep the feasibility condition in one place.- If you care about micro-optimizations, you could short‑circuit the
sum(min(...))loops once the running total meets or exceedsn * target_timeto avoid iterating the entire list for obviously feasible candidates.Functionality-wise this is good to go.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
algorithms/search/binary_search/maxruntime_n_computers/images/examples/max_runtime_n_computers_example_1.pngis excluded by!**/*.pngalgorithms/search/binary_search/maxruntime_n_computers/images/examples/max_runtime_n_computers_example_2.pngis excluded by!**/*.pngalgorithms/search/binary_search/maxruntime_n_computers/images/examples/max_runtime_n_computers_example_3.pngis excluded by!**/*.pngalgorithms/search/binary_search/maxruntime_n_computers/images/solution/max_runtime_n_computers_solution_1.pngis excluded by!**/*.pngalgorithms/search/binary_search/maxruntime_n_computers/images/solution/max_runtime_n_computers_solution_2.pngis excluded by!**/*.pngalgorithms/search/binary_search/maxruntime_n_computers/images/solution/max_runtime_n_computers_solution_3.pngis excluded by!**/*.pngalgorithms/search/binary_search/maxruntime_n_computers/images/solution/max_runtime_n_computers_solution_4.pngis excluded by!**/*.pngalgorithms/search/binary_search/maxruntime_n_computers/images/solution/max_runtime_n_computers_solution_5.pngis excluded by!**/*.pngalgorithms/search/binary_search/maxruntime_n_computers/images/solution/max_runtime_n_computers_solution_6.pngis excluded by!**/*.pngalgorithms/search/binary_search/maxruntime_n_computers/images/solution/max_runtime_n_computers_solution_7.pngis excluded by!**/*.pngalgorithms/search/binary_search/maxruntime_n_computers/images/solution/max_runtime_n_computers_solution_8.pngis excluded by!**/*.pngalgorithms/search/binary_search/maxruntime_n_computers/images/solution/max_runtime_n_computers_solution_9.pngis excluded by!**/*.png
📒 Files selected for processing (4)
DIRECTORY.md(1 hunks)algorithms/search/binary_search/maxruntime_n_computers/README.md(1 hunks)algorithms/search/binary_search/maxruntime_n_computers/__init__.py(1 hunks)algorithms/search/binary_search/maxruntime_n_computers/test_max_runtime.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
algorithms/search/binary_search/maxruntime_n_computers/test_max_runtime.py (1)
algorithms/search/binary_search/maxruntime_n_computers/__init__.py (2)
max_runtime(4-33)max_run_time_2(51-73)
🪛 LanguageTool
algorithms/search/binary_search/maxruntime_n_computers/README.md
[grammar] ~43-~43: Ensure spelling is correct
Context: ...2. While left < right: - Calculate mid = right - (rught - left) // 2 (biases the midpoint towa...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
DIRECTORY.md
96-96: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
97-97: Unordered list indentation
Expected: 6; Actual: 8
(MD007, ul-indent)
Describe your change:
Finds the max runtime possible for running n computers given a list of batteries
Checklist:
Fixes: #{$ISSUE_NO}.Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.