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

Adding log exception to Mosaic Logger #3089

Merged
merged 7 commits into from
Mar 15, 2024
Merged

Conversation

jjanezhang
Copy link
Contributor

@jjanezhang jjanezhang commented Mar 5, 2024

Adding log exception to Mosaic Logger

Adding function to log exceptions to run metadata

Being used in foundry to catch custom errors
image

Manually tested:
image

Related PR: mosaicml/llm-foundry#1014

@jjanezhang jjanezhang changed the title Adding failure reason log wrapper Adding log exception to Mosaic Logger Mar 7, 2024
@jjanezhang jjanezhang self-assigned this Mar 7, 2024
@jjanezhang jjanezhang marked this pull request as ready for review March 8, 2024 00:08
@jjanezhang jjanezhang requested a review from a team as a code owner March 8, 2024 00:08
@eracah
Copy link
Contributor

eracah commented Mar 8, 2024

Can we do some sort of manual test to see how the exception looks when it's logged to mcloud?

Copy link
Contributor

@mvpatel2000 mvpatel2000 left a comment

Choose a reason for hiding this comment

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

How is log exception called?

Copy link
Contributor

@eracah eracah left a comment

Choose a reason for hiding this comment

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

whoa that manual test example is pretty cool. I like that it shows up really nicely in the describe run output. LGTM. Thanks, @jjanezhang !

@jjanezhang jjanezhang merged commit 2f11230 into dev Mar 15, 2024
14 checks passed
@jjanezhang jjanezhang deleted the jane/update-mosaicml-logger branch March 15, 2024 16:09
j316chuck pushed a commit that referenced this pull request May 16, 2024
* adding failure reason log wrapper

* added test and exception to json

* removed non custom class vars from run metadata exc

* force flush exceptions
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