-
Notifications
You must be signed in to change notification settings - Fork 0
Remote Training Progress Reporter w/ LossViewer/RTC #4
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?
Conversation
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.
Pull Request Overview
This PR integrates a LossViewer component into the remote training progress reporting workflow using RTC. It enhances the client by adding LossViewer support for displaying progress messages and includes minor version updates.
- Adds LossViewer, QtImageDirectoryWidget, and related training configuration imports.
- Introduces new parameters and logic to update the LossViewer with progress reports from the worker.
- Updates the project version in pyproject.toml.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sleap_client/client.py | Adds LossViewer integration and handles progress report updates via RTC. |
| pyproject.toml | Bumps project version to reflect new release. |
| if win: | ||
| win._check_messages( | ||
| # Progress should be result from jsonpickle.decode(msg_str) | ||
| rtc_msg=progress | ||
| ) | ||
| else: | ||
| logging.info(f"No monitor window available! win is {win}") |
Copilot
AI
Jun 13, 2025
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.
The logic for updating the LossViewer with progress reports is duplicated in both string and bytes message handling blocks; consider refactoring this into a shared helper function to reduce redundancy.
| if win: | |
| win._check_messages( | |
| # Progress should be result from jsonpickle.decode(msg_str) | |
| rtc_msg=progress | |
| ) | |
| else: | |
| logging.info(f"No monitor window available! win is {win}") | |
| _update_loss_viewer(win, progress) |
| reconnecting = False | ||
| reconnect_attempts = 0 | ||
| output_dir = "" | ||
| win = None |
Copilot
AI
Jun 13, 2025
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.
[nitpick] Using a global variable 'win' to manage the LossViewer instance can make the code harder to maintain; consider encapsulating this state within a class or passing it as a parameter to improve modularity.
…iewer