Active thermal monitoring, boundary checks, and merged community fixes #9
Conversation
Couple of changes: - color not working in windows terminal - suffix in filename in case of voltage/frequency argument - limited (1 successive) retries in case of overheat (previously any chip overheat reached ended the benchmark)
Couple of changes: - color not working in windows terminal - timestamp in filename - suffix in filename in case of voltage/frequency argument - filename computation refactored (called twice) - limited (1 successive) retries in case of overheat (previously any chip overheat reached ended the benchmark)
…solve API mapping and initial state issues
This commit introduces significant enhancements to the Bitaxe Hashrate Benchmark script, focusing on collecting more comprehensive performance data and resolving key issues related to API data retrieval and script robustness.
**Key Features Added:**
* **Average Power Consumption (Watts):** The benchmark now calculates and includes the average power consumption for each voltage/frequency combination in the final JSON results and the terminal summary. This provides a direct metric for power efficiency analysis.
* **Average Fan Speed (RPM/Percentage):** Fan speed data is now fetched and averaged for each tested configuration, then included in the final JSON results and terminal summary. This allows for better assessment of cooling performance and noise levels alongside hashing performance.
* **Real-time Power and Fan Speed Display:** The live terminal output during benchmarking now includes the current power consumption and fan speed for each individual sample, enabling immediate monitoring of these critical metrics.
**Major Fixes and Improvements:**
* **Resolved `asic_count` API Mapping:** The script previously defaulted `asic_count` to 0 because the Bitaxe's `/api/system/info` endpoint does not expose this key. `asic_count` is now hardcoded to 1 in the configuration, ensuring the "Expected Hashrate" calculation is accurate for Bitaxe Gamma 601 (BM1370) models.
* **Corrected Fan Speed API Key:** The script's attempt to fetch "fanSpeed" from the API has been corrected to use "fanspeed" (for percentage) or "fanrpm" (for RPM), based on direct API response analysis. This ensures fan speed data is correctly retrieved.
* **Improved `benchmark_iteration` Return Consistency:** The `benchmark_iteration` function's return signature has been standardized to consistently return 8 values (including `average_power`, `average_fan_speed`, and `error_reason`), ensuring proper unpacking in the main loop. All early exit paths within this function now return a consistent tuple length.
* **Enhanced Script Robustness (Addressing NameError and `results` contradiction):**
* Variables like `error_reason`, `avg_power`, `avg_fan_speed`, etc., are now correctly managed and passed throughout the main execution flow.
* Although the previous `NameError` was challenging to pinpoint without a full traceback, the current implementation addresses potential scope ambiguities and ensures the `results` list is correctly populated and handled even during unexpected exits. This should prevent the "results list is empty" contradiction seen previously.
* **Faster Testing Configuration (Optional):** The default `benchmark_time` was adjusted to 120 seconds (2 minutes) and `frequency_increment` adjusted to 25 MHz for quicker test runs during development. (These can be reverted to original for more exhaustive testing).
These changes collectively make the benchmark tool more accurate, informative, and resilient to common API data inconsistencies, providing a richer dataset for Bitaxe optimization.#
This commit introduces a new command-line option to directly set the core voltage and frequency on the Bitaxe miner, bypassing the full benchmarking process. **Motivation:** Previously, applying specific desired settings required either temporarily modifying the script or running a full benchmark starting at those settings. This new functionality provides a quick, convenient, and direct way to configure the miner to a known good state or a specific operating point without needing to run an entire benchmark cycle. This is particularly useful for fine-tuning after initial benchmarks, or for quickly re-applying optimal settings. **Implementation:** A new command-line flag, -sor--set-values, has been added using argparse. When this flag is detected, the script will: 1. Parse the provided core voltage (-v) and frequency (-f) as the target settings. 2. Call the existing set_system_settings() function to apply these parameters to the Bitaxe. 3. Print confirmation messages. 4. Exit immediately using sys.exit(0), preventing the benchmark loop from initiating. **Usage:** To use this new mode, run the script with the --set-values flag, specifying your desired voltage and frequency: python bitaxe_hasrate_benchmark.py <bitaxe_ip> --set-values -v <desired_voltage_mV> -f <desired_frequency_MHz> Example: python bitaxe_hasrate_benchmark.py 192.168.1.136 --set-values -v 1150 -f 780 This enhancement streamlines the process of applying specific configurations to the Bitaxe miner.#
Enhanced the script's command-line help message to be more user-friendly and informative. Key improvements include: * **Clearer Descriptions:** Detailed explanations for each argument, including their dual purpose for benchmarking and setting values. * **Usage Examples:** Added explicit command examples for both benchmark and set-only modes in the help message's epilog. * **Improved Formatting:** Utilized a custom formatter to preserve multi-line text, add default values, and incorporate color for better readability.#
Resolved rendering issues in argument lists by correctly applying single backticks for inline code, ensuring proper display on GitHub.
Resolved rendering issues in argument lists by correctly applying single backticks for inline code, ensuring proper display on GitHub.
|
Warning Rate limit exceeded@cerebrux has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a settings-only CLI mode, ANSI/color support, timestamped result filenames, streaming stddev and per-iteration power/fan metrics, expanded benchmark return values and result structure, enhanced stabilization/restart and safety checks, and a comprehensive README rewrite with Docker and usage examples. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "bitaxe CLI"
participant Logic as "Benchmark Logic"
participant Device as "BitAxe Device"
participant File as "Result File"
User->>CLI: Invoke CLI (benchmark or --set-values)
CLI->>Logic: Parse args, determine mode
alt Settings-only Mode
Logic->>Device: Connect & apply voltage/frequency
Device-->>Logic: Ack / sensor snapshot
Logic->>File: Write result via result_filename()
File-->>Logic: Confirm write
Logic-->>CLI: Exit
else Benchmark Mode
loop For each trial step
Logic->>Device: Apply settings / start miner
Device-->>Logic: Stream hashrate, temps, power, fan
par Stabilization monitoring
Logic->>Device: Query sensors (temp/power/VR)
Device-->>Logic: Sensor data
alt Threshold exceeded
Logic->>Device: Restart or abort
Device-->>Logic: Restart ack
end
and Metrics
Logic->>Logic: Compute averages & running_stddev
Logic->>File: Append iteration result
File-->>Logic: Confirm append
end
end
Logic->>File: Write final report via result_filename()
File-->>Logic: Confirm write
Logic-->>CLI: Exit with report
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
bitaxe_hashrate_benchmark.py (2)
677-679: Redundantrestart_system()call causes double restart.
set_system_settingsalready callsrestart_system()internally (line 310), so calling it again here results in two restarts. The PR description mentions removing redundant restart calls elsewhere, but this one was missed.🔎 Proposed fix
else: print(YELLOW + "No valid benchmarking results found. Applying predefined default settings." + RESET) set_system_settings(default_voltage, default_frequency) - restart_system()
687-689: Same redundantrestart_system()call infinallyblock.Same issue as the
exceptblock—set_system_settingsalready handles the restart internally.🔎 Proposed fix
else: print(YELLOW + "No valid benchmarking results found. Applying predefined default settings." + RESET) set_system_settings(default_voltage, default_frequency) - restart_system()
🧹 Nitpick comments (5)
bitaxe_hashrate_benchmark.py (4)
349-350: Consider optional debug logging for transient errors.The bare
except Exception: passis flagged by static analysis (S110, BLE001), but the pattern is acceptable here since transient connection errors are expected during device restart. For debugging purposes, you might consider adding optional logging:except Exception: - pass # Ignore transient errors during restart + pass # Transient errors expected during restart; optionally log for debugging
536-546: Remove extraneousfprefixes from strings without placeholders.These strings have no format placeholders, so the
fprefix is unnecessary.🔎 Proposed fix
- print(GREEN + f"\n--- Benchmark Complete ---" + RESET) + print(GREEN + "\n--- Benchmark Complete ---" + RESET) print(GREEN + f"Best Hashrate Settings: {best_voltage}mV / {best_frequency}MHz ({best_result['averageHashRate']:.2f} GH/s)" + RESET) print(GREEN + f"Most Efficient Settings: {eff_voltage}mV / {eff_frequency}MHz ({efficient_result['efficiencyJTH']:.2f} J/TH)" + RESET) - print(YELLOW + f"\nWARNING: 'Best Hashrate' settings are often at the thermal/stability limit." + RESET) - print(YELLOW + f"Running these settings 24/7 may reduce hardware lifespan." + RESET) - print(YELLOW + f"Applying 'Most Efficient' settings is generally safer for long-term use." + RESET) + print(YELLOW + "\nWARNING: 'Best Hashrate' settings are often at the thermal/stability limit." + RESET) + print(YELLOW + "Running these settings 24/7 may reduce hardware lifespan." + RESET) + print(YELLOW + "Applying 'Most Efficient' settings is generally safer for long-term use." + RESET) - print(GREEN + f"\nApplying Best Hashrate settings..." + RESET) + print(GREEN + "\nApplying Best Hashrate settings..." + RESET)
12-13: Dead code:START_TIMEcheck is always false.
START_TIMEis already defined at line 9 before this check executes, so the condition'START_TIME' not in globals()is never true. This block can be removed.🔎 Proposed fix
from datetime import datetime START_TIME = datetime.now().strftime("%Y-%m-%d_%H") - - -if 'START_TIME' not in globals(): - START_TIME = datetime.now().strftime("%Y-%m-%d_%H")
773-792: Dead code:cleanup_and_exitis never called.This function is defined at lines 774–792 but never invoked anywhere in the script. The cleanup logic it implements is duplicated in the
handle_sigintfunction (lines 260–278) and thefinallyblock (lines 675–695). Remove this unused function.README.md (1)
11-11: Clarify fan speed units.The code retrieves
fanspeedfrom the API (line 411 in the Python file) and displays it with%suffix (line 453). Consider updating to just "Fan speed monitoring and reporting (Percentage)" unless the API sometimes returns RPM.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.mdbitaxe_hashrate_benchmark.py
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~119-~119: Ensure spelling is correct
Context: ...ion: 30W * Minimum allowed frequency: 400MHz * Minimum input voltage: 4800mV * Maximu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~127-~127: Ensure spelling is correct
Context: ...ncrement: 15mV * Frequency increment: 20MHz * ASIC Configuration: asic_count is hard...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~192-~192: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...tributing** Contributions are welcome! Please feel free to submit a Pull Request. ## License ...
(FEEL_FREE_TO_STYLE_ME)
🪛 Ruff (0.14.10)
bitaxe_hashrate_benchmark.py
23-23: Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without shell
(S605)
23-23: Starting a process with a partial executable path
(S607)
349-350: try-except-pass detected, consider logging the exception
(S110)
349-349: Do not catch blind exception: Exception
(BLE001)
357-357: Consider moving this statement to an else block
(TRY300)
536-536: f-string without any placeholders
Remove extraneous f prefix
(F541)
540-540: f-string without any placeholders
Remove extraneous f prefix
(F541)
541-541: f-string without any placeholders
Remove extraneous f prefix
(F541)
542-542: f-string without any placeholders
Remove extraneous f prefix
(F541)
546-546: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (9)
bitaxe_hashrate_benchmark.py (7)
15-23: Acceptable ANSI fallback pattern.The
os.system("")trick for enabling ANSI escape sequences on Windows is a known workaround and poses minimal risk since the command is empty. The static analysis warnings (S605, S607) are false positives in this context.
185-190: LGTM!Good defensive implementation clamping variance to
0.0to handle potential negative values from floating-point precision errors.
169-173: LGTM with note on global dependencies.The function relies on module-level globals (
bitaxe_ip,timestamp,file_suffix). This works correctly for the current call sites, which are all after initialization.
362-507: Expanded return signature properly handled.The new 9-tuple return signature is consistently applied across all return paths, and the use of
statistics.stdevon line 466 is appropriate for computing sample standard deviation on the trimmed hashrates.
644-667: Good safety improvements for overheat handling.The critical safety check at lines 646-648 correctly prevents voltage increases when overheating at the initial frequency—a scenario where more voltage would only make things worse. The retry limit (
retry_upon_overheat < 1) prevents infinite overheat cycles.
628-634: Good frequency underflow protection.The boundary check correctly prevents frequency from dropping below
min_allowed_frequency(400 MHz), as stated in the PR objectives.
34-46: LGTM!Clean implementation of a custom formatter that combines raw text formatting with default value display.
README.md (2)
109-128: Configuration documentation is accurate and well-organized.Values match the code constants, and the note about
asic_countbeing hardcoded is helpful context.
190-192: LGTM!The Contributing section is clear and welcoming. The "feel free to" phrasing flagged by style checkers is appropriate for community-facing documentation.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
bitaxe_hashrate_benchmark.py (2)
67-72: Missing validation for requiredbitaxe_ipargument.With
nargs='?',bitaxe_ipcan beNone, but line 118 will produce"http://None"which will cause confusing connection errors later. Add validation after parsing to ensure the IP is provided when not using--help.🔎 Proposed fix after line 117
args = parse_arguments() +if args.bitaxe_ip is None: + print(RED + "Error: Bitaxe IP address is required." + RESET) + sys.exit(1) bitaxe_ip = f"http://{args.bitaxe_ip}"
551-560: Missing return value check forset_system_settings.The
set_system_settingsfunction now returns a boolean indicating success, but this mode ignores it. If stabilization fails (e.g., overheat during boot), the user will be told settings were applied when they may not have been.🔎 Proposed fix
if args.set_values: print(GREEN + "\n--- Applying Settings Only ---" + RESET) print(GREEN + f"Applying Core Voltage: {initial_voltage}mV, Frequency: {initial_frequency}MHz to Bitaxe." + RESET) # Call the existing set_system_settings function - set_system_settings(initial_voltage, initial_frequency) - - print(GREEN + "Settings applied. Check your Bitaxe web interface to confirm." + RESET) - sys.exit(0) # Exit the script after applying settings + if set_system_settings(initial_voltage, initial_frequency): + print(GREEN + "Settings applied. Check your Bitaxe web interface to confirm." + RESET) + sys.exit(0) + else: + print(RED + "Failed to apply settings or stabilization failed. Check your Bitaxe." + RESET) + sys.exit(1)
🧹 Nitpick comments (3)
bitaxe_hashrate_benchmark.py (3)
25-26: Timestamp is computed at module-load time.The timestamp is generated once when the module loads. If the script is run multiple times within the same second (e.g., in automated testing), result files will have identical names and overwrite each other.
Consider moving timestamp generation into
result_filename()or deferring it until the first call.🔎 Proposed refactor
-# Compute timestamp for file suffix -timestamp = time.strftime("%Y%m%d-%H%M%S") - # ANSI Color CodesAnd update
result_filename():def result_filename(): # Extract IP from bitaxe_ip global variable and remove 'http://' ip_address = bitaxe_ip.replace('http://', '') - return f"bitaxe_benchmark_results_{ip_address}_{timestamp}{file_suffix}.json" + timestamp = time.strftime("%Y%m%d-%H%M%S") + return f"bitaxe_benchmark_results_{ip_address}_{timestamp}{file_suffix}.json"
327-351: Active thermal monitoring during stabilization looks good, but consider logging suppressed exceptions.The polling loop correctly monitors temp, vrTemp, and power every 5 seconds and aborts if limits are exceeded. However, the
try-except-passat lines 349-350 silently swallows all exceptions during monitoring, which could hide recurring issues.Consider adding minimal logging or counting failures to aid debugging.
🔎 Proposed enhancement
+ error_count = 0 while time.time() - start_wait < sleep_time: try: info = get_system_info() # ... existing checks ... except Exception: - pass # Ignore transient errors during restart + error_count += 1 + if error_count > 10: + print(YELLOW + "Warning: Multiple errors during stabilization monitoring." + RESET) + error_count = 0 # Reset to avoid spam time.sleep(5)Based on learnings, static analysis tools flagged catching broad
Exceptionandtry-except-pass, but this is acceptable for transient restart errors.
536-546: Remove unnecessary f-string prefixes on lines without placeholders.Lines 536, 540, 541, 542, and 546 use f-string syntax but contain no placeholders. Use regular strings instead.
🔎 Proposed fix
- print(GREEN + f"\n--- Benchmark Complete ---" + RESET) + print(GREEN + "\n--- Benchmark Complete ---" + RESET) print(GREEN + f"Best Hashrate Settings: {best_voltage}mV / {best_frequency}MHz ({best_result['averageHashRate']:.2f} GH/s)" + RESET) print(GREEN + f"Most Efficient Settings: {eff_voltage}mV / {eff_frequency}MHz ({efficient_result['efficiencyJTH']:.2f} J/TH)" + RESET) - print(YELLOW + f"\nWARNING: 'Best Hashrate' settings are often at the thermal/stability limit." + RESET) - print(YELLOW + f"Running these settings 24/7 may reduce hardware lifespan." + RESET) - print(YELLOW + f"Applying 'Most Efficient' settings is generally safer for long-term use." + RESET) + print(YELLOW + "\nWARNING: 'Best Hashrate' settings are often at the thermal/stability limit." + RESET) + print(YELLOW + "Running these settings 24/7 may reduce hardware lifespan." + RESET) + print(YELLOW + "Applying 'Most Efficient' settings is generally safer for long-term use." + RESET) - print(GREEN + f"\nApplying Best Hashrate settings..." + RESET) + print(GREEN + "\nApplying Best Hashrate settings..." + RESET)As per coding guidelines, static analysis flagged these as unnecessary f-strings (F541).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bitaxe_hashrate_benchmark.py
🧰 Additional context used
🪛 Ruff (0.14.10)
bitaxe_hashrate_benchmark.py
23-23: Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without shell
(S605)
23-23: Starting a process with a partial executable path
(S607)
349-350: try-except-pass detected, consider logging the exception
(S110)
349-349: Do not catch blind exception: Exception
(BLE001)
357-357: Consider moving this statement to an else block
(TRY300)
536-536: f-string without any placeholders
Remove extraneous f prefix
(F541)
540-540: f-string without any placeholders
Remove extraneous f prefix
(F541)
541-541: f-string without any placeholders
Remove extraneous f prefix
(F541)
542-542: f-string without any placeholders
Remove extraneous f prefix
(F541)
546-546: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (9)
bitaxe_hashrate_benchmark.py (9)
34-47: LGTM!The custom formatter correctly extends
RawTextHelpFormatterto include default values in help text while respectingargparse.SUPPRESS.
123-158: Configuration and validation checks look good.The configuration properly defines safety limits, and the validation checks ensure initial values are within bounds. The
min_allowed_frequency = 400prevents underflow as intended.
185-191: LGTM! Variance clamping prevents floating-point errors.The
max(var, 0.0)on line 188 correctly guards against tiny negative values caused by floating-point precision issues, preventingValueErrorfrom the square root operation.
300-314: LGTM! Function now returns bool to indicate success.The function correctly returns the result from
restart_system()orFalseon exception, allowing callers to detect stabilization failures.
362-508: LGTM! Benchmark function correctly returns enhanced metrics.The function now returns a 9-tuple including
hashrate_stdev,avg_power,avg_fan_speed, anderror_reason. All early-return paths consistently return the same tuple structure with appropriate error reasons.The running standard deviation is displayed during iteration, and the final
statistics.stdevprovides an accurate result.
644-648: Excellent safety check: prevents voltage increase when overheating at initial frequency.This critical check correctly identifies when the device overheats at the starting frequency and prevents increasing voltage, which would only make it hotter. This is a key safety feature mentioned in the PR objectives.
628-636: Frequency adjustment correctly respects minimum boundary.The logic ensures that frequency never drops below
min_allowed_frequency(400 MHz), preventing underflow as intended in the PR objectives. If the frequency would go below the minimum, it's clamped to the minimum value.
579-591: Stabilization failure handling looks good.The code correctly checks the return value from
set_system_settingsand treats stabilization failures as benchmark failures with an appropriate error reason.
696-770: LGTM! Final reporting correctly includes all new metrics.The results dictionary and final output properly include
hashrateStdDev,averagePower, and optionallyaverageFanSpeedandaverageVRTemp. The conditional inclusion prevents errors when fields are not available.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
README.md (1)
82-85: Documentation is now consistent with code implementation.The code at lines 123-126 of
bitaxe_hashrate_benchmark.pycorrectly validates that both-v/--voltageand-f/--frequencyare required in--set-valuesmode, matching the documentation here.
🧹 Nitpick comments (1)
bitaxe_hashrate_benchmark.py (1)
338-363: Consider logging exceptions during stabilization monitoring.The bare
except Exception: passat lines 361-362 silently swallows all errors during stabilization. While transient network errors are expected during restart, silently ignoring all exceptions could hide real issues (e.g., programming errors, unexpected API responses).🔎 Proposed refinement
- except Exception: - pass # Ignore transient errors during restart + except (requests.exceptions.RequestException, ValueError, KeyError) as e: + # Ignore transient errors during restart (network issues, missing fields) + passThis approach catches expected errors while allowing unexpected exceptions to surface.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.mdbitaxe_hashrate_benchmark.py
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~119-~119: Ensure spelling is correct
Context: ...ion: 30W * Minimum allowed frequency: 400MHz * Minimum input voltage: 4800mV * Maximu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~127-~127: Ensure spelling is correct
Context: ...ncrement: 15mV * Frequency increment: 20MHz * ASIC Configuration: asic_count is hard...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~192-~192: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...tributing** Contributions are welcome! Please feel free to submit a Pull Request. ## License ...
(FEEL_FREE_TO_STYLE_ME)
🪛 Ruff (0.14.10)
bitaxe_hashrate_benchmark.py
19-19: Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without shell
(S605)
19-19: Starting a process with a partial executable path
(S607)
361-362: try-except-pass detected, consider logging the exception
(S110)
361-361: Do not catch blind exception: Exception
(BLE001)
369-369: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (7)
bitaxe_hashrate_benchmark.py (7)
115-119: LGTM: IP validation addresses previous review feedback.The validation correctly ensures
bitaxe_ipis not None before constructing the URL, preventing confusing connection errors.
197-202: LGTM: Robust running standard deviation with numerical stability.The variance clamping at line 200 (
max(var, 0.0)) correctly preventsValueErrorfrom tiny negative floating-point errors.
374-519: LGTM: Enhanced benchmark iteration with comprehensive metrics and error tracking.The updated return signature now includes hashrate standard deviation, average power, average fan speed, and specific error reasons. All call sites at line 605 correctly unpack the 9-value tuple. The streaming stddev calculation (lines 434-435, 445) and final statistics.stdev (line 478) provide robust variability metrics.
564-574: LGTM: Return value check addresses previous review feedback.The
--set-valuesmode now correctly checks the return value fromset_system_settingsand exits with an appropriate error message and status code if stabilization fails.
658-662: LGTM: Critical safety check prevents unsafe voltage increases.This check correctly identifies when the system overheats at the initial frequency and stops further testing. Increasing voltage when already overheating at the baseline would only worsen the thermal condition—excellent safety logic.
643-648: LGTM: Frequency boundary enforcement prevents underflow.The
min_allowed_frequencycheck correctly prevents the frequency from dropping below 400 MHz during adjustment, addressing the boundary check mentioned in the PR objectives.
710-743: LGTM: Result structure properly includes new metrics.The final data structure correctly incorporates
hashrateStdDev,averagePower, and conditionally includesaverageFanSpeedandaverageVRTempwhen available. The conditional unpacking pattern at lines 723-724 and 738-739 handles optional fields elegantly.
4 participants
This PR introduces active polling during the stabilization phase and enforces stricter boundary checks for hardware parameters. It also incorporates changes from PRs #3, #5, and #8.
Features
time.sleep()with a non-blocking polling loop that queries/api/system/infoevery 5 seconds.if tempvrTemporpowerexceed their respective maximums during the 90s wait period.min_allowed_frequency(400MHz) check in the frequency adjustment logic to prevent setting invalid frequencies.CHIP_TEMP_EXCEEDEDoccurs at theinitial_frequency, preventing unsafe voltage increments on an already overheating device.restart_system()call inreset_to_best_setting(). Previously, this caused a double-reboot cycle during script cleanup becauseset_system_settings()already invokes a restart.running_stddevto0.0to preventValueErroron negative floating-point precision errors resulting in negative variance.--max-temp argument(default: 66°C) toargparse.restart_system()call incleanup_and_exitto prevent double-reboot loops.averagePower,averageFanSpeed, andhashrateStdDevto benchmark_iteration return values and JSON output.example new JSON:
{ "coreVoltage": 1150, "frequency": 610, "averageHashRate": 1238.2896082235293, "hashrateStdDev": 23.12398722416522, "averageTemperature": 64.84558823529412, "efficiencyJTH": 15.87926545164911, "averagePower": 19.663129395, "errorReason": null, "averageVRTemp": 75.5, "averageFanSpeed": 31.77805896 },Validation:
originand the new JSON that incorporatesnew featuresresults on the same hardwareORIGIN-bitaxe_benchmark_results.json
NEW-FEATURES-bitaxe_benchmark_result.json
Summary by CodeRabbit
Documentation
New Features
Bug Fixes / Stability
✏️ Tip: You can customize this high-level summary in your review settings.