fixes for gpu monitoring#5
Conversation
Root Cause: - The ui.select component was initialized with an empty options list but with value=0, causing ValueError since 0 is not in the options - NiceGUI validates that initial value must exist in the options list Changes: - Removed the invalid value=0 parameter from ui.select initialization - The value is set properly later in update_gpu_metrics() when GPUs are detected - Added explicit .update() call when setting GPU select value - Improved handling of the no-GPU case to prevent empty select errors Impact: - Monitor page now loads correctly without crashing - GPU selector populates dynamically when GPUs are detected - Better error handling when no GPUs are available
Changes: - Added validation to ensure selected GPU index exists in current GPU list before attempting to update charts - Prevents chart updates when no valid GPU is selected - Returns early if selected GPU is invalid/missing Prevents potential errors when: - Initial selected_gpu_index value (0) doesn't match any GPU - Previously selected GPU becomes unavailable - GPU select component has no value set This addresses user concern #5 about preventing chart updates without a valid GPU selection.
There was a problem hiding this comment.
Pull Request Overview
This PR improves GPU selection handling in the monitor page to prevent errors when no GPUs are available or when the selected GPU becomes invalid. The changes add defensive checks and proper state management for GPU availability.
Key changes:
- Removed default value initialization for the GPU selector to avoid selecting non-existent GPUs
- Added validation to ensure selected GPU exists before updating charts
- Implemented proper handling for GPU disconnection scenarios
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| available_gpus["list"] = gpu_options | ||
| gpu_select.options = gpu_options | ||
| # Only set value if we have GPUs and no valid selection | ||
| if selected_gpu_index["value"] not in [g.index for g in gpus]: |
There was a problem hiding this comment.
The list comprehension [g.index for g in gpus] is created twice in quick succession (lines 123 and 142). Consider storing it in a variable gpu_indices before line 123 to avoid redundant iteration through the GPU list.
| # No GPUs detected | ||
| if available_gpus["list"]: # Had GPUs before but now gone | ||
| available_gpus["list"] = [] | ||
| gpu_select.options = [] |
There was a problem hiding this comment.
When clearing GPU state due to no GPUs detected, selected_gpu_index['value'] should also be reset to maintain consistent state. Consider setting it to None or 0 to prevent using a stale GPU index value.
| gpu_select.options = [] | |
| gpu_select.options = [] | |
| selected_gpu_index["value"] = None |
|
Automated review 🤖 Summary of Changes Key Changes & Positives
Potential Issues & Recommendations
Language/Framework Checks
Security & Privacy
Build/CI & Ops
Tests
Approval Recommendation
|
No description provided.