-
Notifications
You must be signed in to change notification settings - Fork 239
wandb weave tracing integration #270
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
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
…nto rop/weave idk
atroposlib/envs/README.md
Outdated
|
|
||
| Environments emit Weave traces to help you inspect rollout flow and LLM calls: | ||
|
|
||
| - Enabled by default; disable with `WEAVE_DISABLED=true`. |
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.
is it possible to put this into the config dict?
| return "atropos" | ||
|
|
||
|
|
||
| def ensure_weave_init() -> None: |
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.
shouldn't this be populated by the group name?
for more information, see https://pre-commit.ci
…h/atropos into rop/zmq-message-pass
for more information, see https://pre-commit.ci
ZMQ Message Passing & Env Data Aggregation for WandB
|
@ropresearch can you merge in main? |
atroposlib/envs/base.py
Outdated
| # Log with the base environment name | ||
| # allows overlaying all instances on one plot. | ||
| unprefixed_metrics[ | ||
| f"{self.config.desired_name}/{short_key}" |
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.
this isn't a real key
atroposlib/api/sidecar.py
Outdated
|
|
||
| # Otherwise treat as log payload | ||
| if wandb.run is not None: | ||
| wandb.log(payload) |
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.
this should not be controlled here, it should be routed back to the environment
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.
you will also need logic to wait for all connected environments to figure out when to send it back
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
PR Type
📝 General Information
Description
Related Issues
Type of Change
✅ Developer & Reviewer Checklist