-
Notifications
You must be signed in to change notification settings - Fork 325
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
feat: Disable Sagemaker endpoint (or cross-encoder per workspace) #588
Merged
charles-marion
merged 28 commits into
aws-samples:main
from
charles-marion:disable_sagemaker
Oct 21, 2024
Merged
feat: Disable Sagemaker endpoint (or cross-encoder per workspace) #588
charles-marion
merged 28 commits into
aws-samples:main
from
charles-marion:disable_sagemaker
Oct 21, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t the models available need to be hosted on Sagemaker which increases cost significantly. Having an option to disable cross encoder models would be helpful while exploring the chatbot so that Sagemaker costs can be avoided. Added a config to enable/disable cross encoder models. Also added options to selected embedding models, so that sagemakerModels are not created automatically. Persisted enableSagemakerModels config so that it can be used directly instead of relying on sagemakerModels length.
Added basic feedback mechanism for responses generated by the chatbot. The feedbacks are stored in DynamoDB which can be queried to do analysis as required by admin users. In future we can add a UI page to display the feedbacks, but for now these are being stored and manual analysis would be required. The feedbacks are not adding to the learning of the chatbot.
This reverts commit 1c3b8ce.
Hybrid Search won't be available if cross encoding is not enabled.
…iaSagemaker config.
Default embeddings models was not being set correctly. Also error was thrown related to suppression rules if sagemaker models were not enabled. Used props.config.llms.enableSagemakerModels config to add the NAG suppression rules.
Also removed duplicated config.
…t into disable_sagemaker
…t into disable_sagemaker
bigadsoleiman
approved these changes
Oct 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
#222
Description of changes:
This change is a follow up of #286 by @azaylamba
Currently cross encoder models are used to rank the search results but the models available need to be hosted on Sagemaker which increases cost significantly. Having an option to disable cross encoder models would be helpful while exploring the chatbot so that Sagemaker costs can be avoided.
Changes:
Thank you @azaylamba for your help with this change.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.