Support subloss logging through loss dicts #111
Conversation
dosubot
bot
added
the
size:M
WalkthroughThe recent updates enhance the handling of losses in the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- lighter/system.py (1 hunks)
Additional comments not posted (2)
lighter/system.py (2)
217-223: LGTM! The handling of loss as either a tensor or a dictionary is well-implemented.
244-250: Effective handling of different loss types in logging.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Out of diff range and nitpick comments (1)
lighter/system.py (1)
260-261: Optimize logging of optimizer statistics.Consider logging optimizer statistics only once per epoch to reduce the overhead and improve performance. This can be achieved by moving the logging logic to a separate method that is called only once per epoch.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- lighter/system.py (1 hunks)
Additional comments not posted (2)
lighter/system.py (2)
208-210: Ensure robust error handling for loss dictionaries.Consider enhancing the error message to guide the user on how to properly structure the loss dictionary. For example:
- raise ValueError("The loss dictionary must include a 'total' key that combines all sublosses. Example: {'total': combined_loss, 'subloss1': loss1, ...}") + raise ValueError("The loss dictionary must include a 'total' key that combines all sublosses. Example: {'total': combined_loss, 'subloss1': loss1, ...}")
253-255: Ensure consistent metric logging.Consider adding a check to ensure that each metric in the metrics dictionary is an instance of
torchmetrics.Metricto prevent runtime errors during logging:+ if not isinstance(metric, Metric): + raise TypeError(f"Expected type for metric is 'Metric', got '{type(metric).__name__}' instead.")
| for name, optimizer_stat in get_optimizer_stats(self.optimizer).items(): | ||
| on_epoch_log(f"{mode}/{name}", optimizer_stat) |
There was a problem hiding this comment.
Ensure robust configuration of optimizers.
Consider adding error handling for cases where the scheduler is not specified but is expected to be used. This can help prevent runtime errors and improve the robustness of the system.
Refactor dynamic method setup.
Consider refactoring the dynamic setup of methods to improve readability and maintainability. This can be achieved by separating the logic for each stage into separate methods and calling these methods from the setup method.
Description
Related Issue
Type of Change
Checklist
CODE_OF_CONDUCT.mddocument.CONTRIBUTING.mdguide.make codestyle.Summary by CodeRabbit