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

Ignore mosaicml logger for exception if excephook is active #1301

Merged
merged 10 commits into from
Jun 24, 2024

Conversation

jjanezhang
Copy link
Contributor

@jjanezhang jjanezhang commented Jun 23, 2024

Ignore mosaicml logger for exception if excephook is active

Before we remove the logger all together (re: #1292) we want to first test catching exceptions with a flag. This prevents us from logging an exception twice. I will run tests that turn the logger off in favor of overriding the excepthook to make sure the intended before still holds for finetuning.

Ran a test on dbx-staging to confirm that the flag works. We will have to update the foundry image used for finetuning to incorporate this change.
image

@jjanezhang jjanezhang requested a review from a team as a code owner June 23, 2024 23:22
@jjanezhang jjanezhang assigned jjanezhang and dakinggg and unassigned dakinggg Jun 23, 2024
@jjanezhang jjanezhang requested a review from dakinggg June 24, 2024 00:00
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.

there are more places to do this, can you grep foundry for .log_exception and replace all of them (unless there was a reason you only did some)?

@jjanezhang jjanezhang requested a review from dakinggg June 24, 2024 18:14
@jjanezhang jjanezhang requested a review from dakinggg June 24, 2024 19:21
@jjanezhang jjanezhang merged commit 21c9e0a into main Jun 24, 2024
11 checks passed
@dakinggg dakinggg deleted the jane/add-excepthook-flag branch August 6, 2024 18:41
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.

None yet

2 participants