Skip to content

Conversation

@maharajamihir
Copy link

added support for multiple loggers:

wandb
image

tensorboard
image

local
image

console
image

This comment was marked as outdated.

maharajamihir and others added 3 commits July 8, 2025 17:03
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@maharajamihir maharajamihir requested a review from Copilot July 8, 2025 15:06
Copy link

Copilot AI left a 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 centralizes logging by introducing a flexible CompositeLogger that supports multiple backends (wandb, TensorBoard, local JSON/PNG, and console) and updating all training scripts to use it.

  • Added utils/logger.py containing BaseLogger and concrete loggers (WandbLogger, TensorboardLogger, LocalLogger, ConsoleLogger, and CompositeLogger).
  • Modified train_tokenizer.py, train_lam.py, and train_dynamics.py to remove direct wandb usage and configure logging via CompositeLogger.
  • Updated each script’s Args to specify log_dir and a list of desired loggers.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
utils/logger.py New module defining an abstract logger interface and four implementations, plus a composite wrapper.
train_tokenizer.py Replaced wandb calls with CompositeLogger; changed CLI args to loggers list instead of a flag.
train_lam.py Same updates as tokenizer training: use CompositeLogger and new loggers/log_dir args.
train_dynamics.py Aligned with other scripts to use CompositeLogger; removed direct wandb integration.
Comments suppressed due to low confidence (1)

utils/logger.py:233

  • [nitpick] Using the key "tb" for TensorBoard may be unclear to new users. Consider renaming it to "tensorboard" or providing both aliases for clarity.
                            "tb": TensorboardLogger,

@emergenz
Copy link

emergenz commented Jul 9, 2025

Since we are going to do proper logging, do we want to switch from prints to proper python logging?

Copy link

@emergenz emergenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switch to python logging in consoleLogging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants