-
Notifications
You must be signed in to change notification settings - Fork 30
Add real-time and final hashrate standard deviation calculations to benchmark output #8
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
base: main
Are you sure you want to change the base?
Add real-time and final hashrate standard deviation calculations to benchmark output #8
Conversation
WalkthroughAdds runtime hashrate standard deviation calculations, extends benchmark_iteration to return a new SD value, updates result structures and printed/serialized outputs to include hashrateStdDev, and introduces a cleanup_and_exit function for graceful termination and persistence. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Benchmark Runner
participant Iter as benchmark_iteration
participant SD as running_stddev
participant Persist as Final JSON/Results
Runner->>Iter: start(core_voltage, frequency)
loop samples
Iter->>SD: running_stddev(N, s1, s2)
SD-->>Iter: per-sample SD
Iter-->>Runner: status line with SD
end
Iter-->>Runner: (avg_hashrate, hashrate_stdev, avg_temp, efficiency, within_tol, avg_vr_temp, error_reason)
Runner->>Persist: update all_results/top_performers/most_efficient with hashrateStdDev
alt interruption or exit
Runner->>Runner: cleanup_and_exit(reason)
Runner->>Persist: persist results, reset to best/default
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
🔭 Outside diff range comments (2)
bitaxe_hashrate_benchmark.py (2)
339-344
: Guard against empty VR temperature slice to prevent ZeroDivisionErrorIf vr_temps has 6 or fewer readings, trimmed_vr_temps will be empty and the average calculation will divide by zero.
Apply this diff:
- if vr_temps: - sorted_vr_temps = sorted(vr_temps) - trimmed_vr_temps = sorted_vr_temps[6:] # Remove first 6 elements only - average_vr_temp = sum(trimmed_vr_temps) / len(trimmed_vr_temps) + if vr_temps: + sorted_vr_temps = sorted(vr_temps) + trimmed_vr_temps = sorted_vr_temps[6:] # Remove first 6 elements only + if trimmed_vr_temps: + average_vr_temp = sum(trimmed_vr_temps) / len(trimmed_vr_temps) + else: + average_vr_temp = None
546-566
: Deduplicate cleanup paths: integrate cleanup_and_exit into signal and shutdown flowcleanup_and_exit encapsulates the reset/save/exit flow but isn’t used. Today, similar logic exists in handle_sigint and the finalization block, leading to duplication and risk of divergence.
Proposed integration:
- In handle_sigint, delegate to cleanup_and_exit to centralize behavior.
- In the finalization path, call cleanup_and_exit() once rather than reimplementing the logic.
Example replacement for handle_sigint:
def handle_sigint(signum, frame): global handling_interrupt if handling_interrupt or system_reset_done: return handling_interrupt = True print(RED + "Benchmarking interrupted by user." + RESET) cleanup_and_exit(reason="Interrupted by user")And in the finalization section, replace the manual reset/save/restart logic with a single call:
finally: cleanup_and_exit()Note: cleanup_and_exit already guards with system_reset_done to ensure idempotency.
🧹 Nitpick comments (3)
bitaxe_hashrate_benchmark.py (3)
244-246
: Consider Welford’s algorithm for numerically stable running SDThe s1/s2-based formula is acceptable with the clamp above, but Welford’s algorithm avoids catastrophic cancellation entirely and is the standard for online variance/SD.
Apply these diffs to switch to Welford’s algorithm:
- s1 = 0.0 - s2 = 0.0 + mean = 0.0 + M2 = 0.0- s1 += hash_rate - s2 += hash_rate * hash_rate + # Welford's online update + n = sample + 1 + delta = hash_rate - mean + mean += delta / n + delta2 = hash_rate - mean + M2 += delta * delta2- running_sd = running_stddev(sample + 1, s1, s2) + running_sd = (M2 / (n - 1)) ** 0.5 if n > 1 else 0.0Also applies to: 297-300, 307-307
314-314
: Increase visibility of SD with 1 decimal place in status lineShowing one decimal improves readability without clutter.
Apply this diff:
- f"SD: {running_sd:3.0f} GH/s | " + f"SD: {running_sd:5.1f} GH/s | "
445-445
: Typo: “to low” → “too low”Minor grammar fix for user-facing output.
Apply this diff:
- print(YELLOW + f"Hashrate to low compared to expected. Decreasing frequency to {current_frequency}MHz and increasing voltage to {current_voltage}mV" + RESET) + print(YELLOW + f"Hashrate too low compared to expected. Decreasing frequency to {current_frequency}MHz and increasing voltage to {current_voltage}mV" + RESET)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
bitaxe_hashrate_benchmark.py
(11 hunks)
🔇 Additional comments (6)
bitaxe_hashrate_benchmark.py (6)
7-7
: Import of statistics is appropriateImporting statistics to compute the final stdev on the trimmed hashrates is correct and aligns with the new functionality.
256-256
: Return arity consistently updated to 7-tupleAll early-exit paths and final returns now consistently return seven values, matching the updated unpacking at the call site. Good attention to consistency.
Also applies to: 263-263, 267-267, 272-272, 276-276, 280-280, 284-284, 291-291, 295-295, 352-352, 364-364, 367-367
331-331
: Final stdev on trimmed set looks correctUsing statistics.stdev on the trimmed hashrates with a safe fallback when only one sample remains is correct.
358-358
: Printing final hashrate Std Dev is correctAdding the final “Hashrate Std Dev” printout aligns with the new metric and aids quick inspection.
415-415
: Caller unpack updated 7-tuple correctlybenchmark_iteration now returns hashrate_stdev as the second item. The caller’s unpack matches.
422-422
: Propagating hashrateStdDev to results, rankings, and prints is correctThe new hashrateStdDev field is persisted, included in final JSON, and printed in both top-5 sections. Use of get with a default when printing is a safe fallback.
Also applies to: 490-490, 503-503, 526-526, 538-538
def running_stddev(N, s1, s2): | ||
if N > 1: | ||
return ((N * s2 - s1 ** 2) / (N * (N - 1))) ** 0.5 | ||
else: | ||
return 0.0 | ||
|
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.
Fix numerical instability in running_stddev to avoid complex results
Floating-point round-off can make (N*s2 - s1**2) slightly negative, which will produce a complex result when taking the square root and will break f-string formatting. Clamp negative variance to zero.
Apply this diff:
-def running_stddev(N, s1, s2):
- if N > 1:
- return ((N * s2 - s1 ** 2) / (N * (N - 1))) ** 0.5
- else:
- return 0.0
+def running_stddev(N, s1, s2):
+ if N <= 1:
+ return 0.0
+ var = (N * s2 - s1 * s1) / (N * (N - 1))
+ if var < 0:
+ # Guard against tiny negative values from floating-point round-off
+ var = 0.0
+ return var ** 0.5
Optional: For better numerical stability, consider replacing the sum/sum-of-squares approach with Welford’s one-pass algorithm (see separate comment).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
def running_stddev(N, s1, s2): | |
if N > 1: | |
return ((N * s2 - s1 ** 2) / (N * (N - 1))) ** 0.5 | |
else: | |
return 0.0 | |
def running_stddev(N, s1, s2): | |
if N <= 1: | |
return 0.0 | |
var = (N * s2 - s1 * s1) / (N * (N - 1)) | |
if var < 0: | |
# Guard against tiny negative values from floating-point round-off | |
var = 0.0 | |
return var ** 0.5 |
🤖 Prompt for AI Agents
In bitaxe_hashrate_benchmark.py around lines 93 to 98, the running_stddev
function can produce a tiny negative value inside the square root due to
floating-point round-off; compute the variance as (N * s2 - s1 ** 2) / (N * (N -
1)), clamp it to zero with max(var, 0.0) before taking the square root to avoid
complex results, and return math.sqrt(var) (or var ** 0.5) of the non-negative
value.
This update enhances the benchmarking script by adding both running and final standard deviation calculations for hashrate measurements.
Key changes include:
This provides more insight into hashrate variability during and after benchmarking.
Summary by CodeRabbit