-
Notifications
You must be signed in to change notification settings - Fork 224
fix unexpected keyword argument render_options #52 #54
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?
fix unexpected keyword argument render_options #52 #54
Conversation
0158917 to
8ce167b
Compare
chris-monardo
left a comment
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.
Thanks!
|
This is one of the 30 or so substantive challenges I am working through, quite a grind, on getting the 120B actually running in this system. It is a ton of work. I think I have the harmony conversion done, the weight mapping done, have def sorted the keys on both sides for the weight mapping, but getting them patched in is still a grind. I'm on it. Not sure if the authors prefer I fork or run a pull request here, let me know. |
|
I don't see an issue with this PR but I can't replicate the issue this is meaning to fix. Do you have a test or an example to reproduce the fix? I tried to |
|
Python 3.13.5 |
|
taking a look today, thanks! |
scott-oai
left a comment
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 am assuming you are only seeing this while doing local development? i have not been able to replicate this in CI or with a clean build. i was able to replicate this in an old version of the repo, which was used stale build.
locally if you run rm -f python/openai_harmony/openai_harmony.cpython-*.so and then rerun tests you should be good.
let me know if that fixes it or if there is still a remaining issue
|
I updated the test script do this in https://github.com/openai/harmony/pull/85/files#diff-290b975da1856c41de1e209d4bd830231577ddfc6608c94fd01baea8ac164540 |
The Python wrapper called the Rust binding with keyword arguments, but the Rust binding expected positional arguments. That's why #52 reported the call to
encoding.render(msg, render_options=...). So, I changed the Python method to pass positional arguments instead of keyword arguments when calling the binding.