-
Notifications
You must be signed in to change notification settings - Fork 53
feat: Updates for intrinsics support #227
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
Pull changes from Jal/intrinsic updates
Pull additional updates from Jal/intrinsic branch
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
jakelorocco
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.
looks mostly good to me! I left a few comments and we should definitely let others review as well (I will bring this up in our team meeting Wednesday).
Also, it looks like the pre-commit checks are failing. I think it's an issue from my base branch, but could you please run pre-commit run --files mellea/backends/openai.py. Or you can just move from enum import Enum below from copy import deepcopy in that file.
Once that gets fixed, we can make sure the github runners can run the new tests (else we'll have to mark them as qualitative). Thank you!
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
|
FYI @pronics2004 |
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
mellea/stdlib/intrinsics/rag.py
Outdated
| base_model_name = backend.model_id | ||
| if base_model_name is None: | ||
| raise ValueError("Backend has no model ID") | ||
| adapter = GraniteCommonAdapter( | ||
| repo_id, intrinsic_name, adapter_type, base_model_name=base_model_name | ||
| ) |
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.
After the discussion on other comments, I also realize a difference here. Because we are no longer creating the adapter config in the backend, the base_model_name must be known here (when the Backend could be any type of backend).
Let me investigate a solution, but backend.model_id can be of types str | ModelIdentifier. At the very least, we should handle some basic version of the ModelIdentifier case. I was splitting on the .hf_model_name but that might not be the best assumption at this point.
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
jakelorocco
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.
@frreiss, I also have a few changes related to my previous comments. How would you like to handle those? I can fork your fork and open up a PR, you can give me access to your repo, or I can send you the git diff.
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
@frreiss, I opened up this fork and pull request with my changes: frreiss#3. Please let me know if you see any issues / would prefer I merge these changes differently. Otherwise, please merge if you are okay with them. |
feat: small changes to intrinsics; along with fixes to docs / tests
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
Signed-off-by: Fred Reiss <frreiss@us.ibm.com>
|
This PR should be ready to merge now, but the CI workflow keeps canceling itself after 7 minutes. |
This PR includes changes from @jakelorocco that implement intrinsics support, plus additional changes that I have made to enable the RAG intrinsics to work.
Jacob's changes:
GraniteCommonAdapterclass and supporting classesMy additional changes: