Skip to content

Conversation

@flaviabeo
Copy link
Contributor

No description provided.

Copy link
Collaborator

@evaline-ju evaline-ju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding examples! Some suggestions + looks like formatting is failing

"model_id": model_id
}
response = requests.post(
f"http://{host}:8080/api/v1/task/rerank-tasks",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The host variable is only present under the grpc section - we may want to move that variable out and not make this part conditional on grpc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed this to another file - let me know if that was the suggestion?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah my suggestion wasn't particularly that this had to be a separate file. Previously, the host variable was only under the if get_config().runtime.grpc.enabled: so if that had been disabled, this http portion would've errored. As long as this code runs in all known cases and is easy to follow for a user, I am not particular on file placement 😄

Copy link
Collaborator

@evaline-ju evaline-ju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nit, assuming all the notebooks and scripts have been run again and confirmed to work with the recent changes

"model_id": model_id
}
response = requests.post(
f"http://{host}:8080/api/v1/task/rerank-tasks",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah my suggestion wasn't particularly that this had to be a separate file. Previously, the host variable was only under the if get_config().runtime.grpc.enabled: so if that had been disabled, this http portion would've errored. As long as this code runs in all known cases and is easy to follow for a user, I am not particular on file placement 😄

request, metadata=[("mm-model-id", model_id)], timeout=1
)

# print("RESPONSE:", response)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: removable?

Suggested change
# print("RESPONSE:", response)
# print("RESPONSE:", response)

os.environ['ALLOW_DOWNLOADS'] = "1"

import caikit_nlp
model_name = "BAAI/bge-large-en-v1.5"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change the model name to "sentence-transformers/all-MiniLM-L6-v2" just so that the person who follows this tutorial can mostly copy and paste the commands?

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 this pull request may close these issues.

3 participants