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.

Sign up for GitHub

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

Jump to bottom

Adding Mosaic logger + logging data validated event #670

Merged
merged 10 commits into from
Oct 20, 2023
Merged

Adding Mosaic logger + logging data validated event #670

merged 10 commits into from
Oct 20, 2023

Conversation

jjanezhang
Copy link
Contributor

@jjanezhang jjanezhang commented Oct 12, 2023 •
edited
Loading

Adding Mosaic Logger + data validated log #670

  1. Added mosaic logger to log foundry information for each run.
  2. Logged the timestamp for data validation complete to mosaic logger

Testing

  1. Started run from MCLI using this YAML. Checked run metadata after run finished and confirmed that data_validated timestamp is logged in the run metadata as well as all other composer logged metrics.
image

dakinggg
dakinggg reviewed Oct 16, 2023
View reviewed changes
llmfoundry/callbacks/__init__.py Outdated Show resolved Hide resolved
scripts/train/train.py Outdated Show resolved Hide resolved
scripts/train/train.py Outdated Show resolved Hide resolved
@jjanezhang jjanezhang self-assigned this Oct 17, 2023
dakinggg
dakinggg reviewed Oct 17, 2023
View reviewed changes
Copy link
Collaborator

@dakinggg dakinggg left a comment

Choose a reason for hiding this comment

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

Could you please update the PR description based on the current implementation?

scripts/train/train.py Outdated Show resolved Hide resolved
scripts/train/train.py Show resolved Hide resolved
scripts/train/train.py Outdated Show resolved Hide resolved
irenedea
irenedea reviewed Oct 17, 2023
View reviewed changes
scripts/train/train.py Outdated Show resolved Hide resolved
irenedea
irenedea reviewed Oct 17, 2023
View reviewed changes
scripts/train/train.py Outdated Show resolved Hide resolved
@jjanezhang jjanezhang changed the title Adding Mosaic Logger + Run Events Callback Adding Mosaic Logger + data validated log Oct 18, 2023
@jjanezhang jjanezhang changed the title Adding Mosaic Logger + data validated log Adding Mosaic logger + logging data validated event Oct 18, 2023
irenedea
irenedea approved these changes Oct 18, 2023
View reviewed changes
Copy link
Contributor

@irenedea irenedea left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀 🔥

dakinggg
dakinggg reviewed Oct 18, 2023
View reviewed changes
scripts/train/train.py Outdated Show resolved Hide resolved
dakinggg
dakinggg reviewed Oct 18, 2023
View reviewed changes
Copy link
Collaborator

@dakinggg dakinggg left a comment

Choose a reason for hiding this comment

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

Before merging, can you please paste into the PR description evidence of a manual test that successfully logged the metric you are interested in?

irenedea
irenedea reviewed Oct 19, 2023
View reviewed changes
scripts/train/train.py Outdated Show resolved Hide resolved
@jjanezhang
add check for missing mosaicmllogger
d2ccaf5 
fixed typing for mosaicmllogger

fixed formatting, moved logging logic out of logger builder

removed callback moved all to train.py

Convert to DataSpec and add token counts that include padding (#676)

Add support for automatically registering models to UC at the end of training (#618)

removed callback from init

removed callback from init

removed generate callback

sorted imports

formatting changes
@jjanezhang
merge conflicts
831fe24
@jjanezhang
logger adding logic fix
5af0788
@jjanezhang
logger adding logic fix
ba3f8fb
@jjanezhang jjanezhang merged commit 459947c into main Oct 20, 2023
12 checks passed
@dakinggg dakinggg deleted the jane/training-events branch November 17, 2023 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dakinggg dakinggg dakinggg left review comments

@irenedea irenedea irenedea approved these changes

@nancyhung nancyhung Awaiting requested review from nancyhung

Assignees

@jjanezhang jjanezhang

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jjanezhang @irenedea @dakinggg