Skip to content
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

Add resume wandb reporter #293

Merged
merged 4 commits into from
Nov 20, 2024
Merged

Add resume wandb reporter #293

merged 4 commits into from
Nov 20, 2024

Conversation

jewelltaylor
Copy link
Collaborator

PR Type

[Feature | Fix | Documentation | Other ]

Short Description

Small change that adds the ability to specify the behaviour of the wandb reporter when a run of the same entity, project and run id are the same. Defaults to allow which gracefully handles the cases when the it does and does not exist. I need this specifically if we are loading an experiment from a checkpoint and continuing on from there. Let me know if there are any issues!

@jewelltaylor jewelltaylor requested review from scarere, emersodb, lotif, fatemetkl and sanaAyrml and removed request for scarere November 20, 2024 01:08
Copy link
Collaborator

@emersodb emersodb left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@emersodb
Copy link
Collaborator

I would say maybe wait for Shawn to take a quick look though, as he is now the resident W and B expert of our group 😂

@jewelltaylor
Copy link
Collaborator Author

I would say maybe wait for Shawn to take a quick look though, as he is now the resident W and B expert of our group 😂

For sure hahaha! @scarere on that note, are you opposed to me changing fit_round_time_elapsed to a non hidden metric? It would be nice to see that out of the box so we can keep track of how long fit times are taking and if we're seeing a slow down over time

@jewelltaylor
Copy link
Collaborator Author

Added one more commit just changing fit_round_time_elapsed and eval_round_time_elapsed to nonhidden metrics

Copy link
Collaborator

@scarere scarere left a comment

Choose a reason for hiding this comment

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

LGTM

@jewelltaylor jewelltaylor merged commit c8e967f into main Nov 20, 2024
6 checks passed
@jewelltaylor jewelltaylor deleted the add-resume-wandb-reporter branch November 20, 2024 18:07
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