-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
New RM Serper #102
New RM Serper #102
Conversation
Hi @zenith110 , thank you so much for working on this! I will review it and help merge this very useful feature. Before I proceed, could you first revert the change on unnecessary files? I think this PR shall only contain changes to |
I'm curious, what formatters do y'all suggest? I've been using black for personal projects since it works for pep8. |
Thanks, please also revert change to other scripts under |
Good question. Currently, we haven't set up a default one for this project. I think we probably need to set one up (what do you think @Yucheng-Jiang?) - the main purpose is to maintain readability and to avoid changing other unrelated files when contributing a certain feature. |
…/storm into users/zenith110/serper
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.
Thank you so much for your contribution! Overall, it looks good to me, except some nits. Could you help fix them so I could merge this?
Hiya @shaoyijia following up if the changes look good. Thank you. |
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.
Looks good, thank you so much for following up!!
…erper Add support for SerperRM.
This PR aims to accomplish the following:
(important to know that it can also be a list of dictionaries as well to 100 in an list).
example.py is a local file I made in knowledge_storm to test as the example could not pick up the imports for knowledge_storm oddly enough.
I couldn't get the secrets.toml loaded so I set local environment variables.
I also pinned a specific version of NumPy as the latest version(NumPy 2) was having issues with PyTorch, would recommend pinning versions in the future to avoid issues with cross compability.
Looking forward to the feedback.
For input validation, the bare minimum in the serper example provides what would be needed to pass, would more input validation be appropriate?