Skip to content

Conversation

@andy-stark-redis
Copy link
Contributor

@andy-stark-redis andy-stark-redis commented Apr 17, 2025

@andy-stark-redis andy-stark-redis requested a review from a team April 17, 2025 10:38
@andy-stark-redis andy-stark-redis self-assigned this Apr 17, 2025
Copy link
Collaborator

@dwdougherty dwdougherty left a comment

Choose a reason for hiding this comment

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

Just a few things to look at. Otherwise, LGTM. Approving now so you don't get held up.

generates embeddings from text:

```php
$extractor = pipeline('embeddings', 'Xenova/all-MiniLM-L6-v2');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be Xenova/all-mpnet-base-v2?

Suggested change
$extractor = pipeline('embeddings', 'Xenova/all-MiniLM-L6-v2');
$extractor = pipeline('embeddings', 'Xenova/all-mpnet-base-v2');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dwdougherty Actually, this is the only instance that's right! (It works in the tested PHP code.) I accidentally used the wrong model name in the text by taking it from another example that uses a different model :-( Fixed now, though - thanks for spotting the inconsistency.

@andy-stark-redis
Copy link
Contributor Author

@dwdougherty Thanks for the review!

@andy-stark-redis andy-stark-redis merged commit b529ee1 into main Apr 17, 2025
5 checks passed
@andy-stark-redis andy-stark-redis deleted the DOC-4544-php-vec-example branch April 17, 2025 13:48
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