-
Notifications
You must be signed in to change notification settings - Fork 653
Firehose Logging #480
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
Firehose Logging #480
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
olmo/util.py
Outdated
@@ -59,6 +59,7 @@ def __repr__(self) -> str: | |||
class LogFilterType(StrEnum): | |||
rank0_only = "rank0_only" | |||
local_rank0_only = "local_rank0_only" | |||
firehose = "firehose" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion here but maybe "all_ranks" instead of "firehose" to be more consistent with the existing names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in dd454bd.
Tests fail because of a version conflict with cached_path. Cached_path can't be updated because of the script that parses the changelog. Looks like AGI has to wait another day. |
Oyvind has a fix in this PR: #484 |
This is true, but the underlying issue is an unnecessary pin in cached_path. |
No description provided.