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

Wrong prefix for logs in KTOTrainer #1631

Closed
bartoszzuk opened this issue May 8, 2024 · 1 comment · Fixed by #1641
Closed

Wrong prefix for logs in KTOTrainer #1631

bartoszzuk opened this issue May 8, 2024 · 1 comment · Fixed by #1641

Comments

@bartoszzuk
Copy link
Contributor

bartoszzuk commented May 8, 2024

The implementation of log method in KTOTrainer uses wrong prefix eval/ instead of eval_ (similar with train metrics).
This results in names of metrics not being properly processed by logging callbacks (I tested on WandbCallback). The problem is specifically with function rewrite_logs, which expects correct prefixes. This results in metric names in wandb like train/train/rewards/margins or train/eval/rewards/margins (see picture below). If you would like I can submit a pull request for this issue since the fix should be pretty trivial?

def rewrite_logs(d):
    new_d = {}
    eval_prefix = "eval_"
    eval_prefix_len = len(eval_prefix)
    test_prefix = "test_"
    test_prefix_len = len(test_prefix)
    for k, v in d.items():
        if k.startswith(eval_prefix):
            new_d["eval/" + k[eval_prefix_len:]] = v
        elif k.startswith(test_prefix):
            new_d["test/" + k[test_prefix_len:]] = v
        else:
            new_d["train/" + k] = v
    return new_d

Screenshot from 2024-05-08 16-23-54

@kawine
Copy link
Contributor

kawine commented May 10, 2024

thank for catching this @bartoszzuk ! please feel free to open a PR

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 a pull request may close this issue.

2 participants