Initial commit of PGVector documentation #5380
Conversation
WalkthroughThe pull request introduces a comprehensive documentation file for integrating PostgreSQL with PGVector in the FiftyOne framework. The new documentation provides a detailed guide on leveraging PostgreSQL's vector search capabilities for computer vision data. It covers the entire workflow from setting up a PostgreSQL cluster to performing advanced similarity searches, including practical implementation steps, configuration options, and code examples for managing vector indexes and querying data. Additionally, corrections and clarifications have been made to the MongoDB Vector Search Integration documentation. Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
docs/source/integrations/pgvector.rst (6)
56-58: Add security best practices for connection strings.Consider adding security recommendations:
- Use environment variables or secure vaults for credentials
- Include a concrete example with dummy values
- Add a note about connection string security best practices
- export FIFTYONE_DATABASE_NAME=fiftyone - export FIFTYONE_DATABASE_URI='postgresql://[userspec@][hostspec][/dbname][?paramspec]' + # Store credentials securely using environment variables + export FIFTYONE_DATABASE_NAME=fiftyone + # Example connection string (replace with your values): + export FIFTYONE_DATABASE_URI='postgresql://fiftyone_user:your_password@localhost:5432/fiftyone?sslmode=require' + + # Security best practices: + # - Never commit connection strings with credentials + # - Use environment variables or secure vaults + # - Enable SSL/TLS when possible using sslmode=require
89-95: Add error handling to the basic recipe example.The example should demonstrate proper error handling for common scenarios:
- Connection failures
- Invalid embeddings
- Resource cleanup in case of failures
# Steps 2 and 3: Compute embeddings and create a similarity index -pgvector_index = fob.compute_similarity( - dataset, - embeddings="embeddings", # the field in which to store the embeddings - brain_key="pgvector_index", - backend="pgvector", -) +try: + pgvector_index = fob.compute_similarity( + dataset, + embeddings="embeddings", # the field in which to store the embeddings + brain_key="pgvector_index", + backend="pgvector", + ) +except ConnectionError as e: + print(f"Failed to connect to PostgreSQL: {e}") + # Handle connection failure +except ValueError as e: + print(f"Invalid embeddings: {e}") + # Handle invalid embeddings +finally: + # Ensure resources are cleaned up in case of failure + if 'pgvector_index' in locals(): + pgvector_index.cleanup()
127-131: Add specific setup examples for common environments.Consider adding:
- Docker compose example for quick setup
- Cloud-specific setup instructions (AWS RDS, GCP Cloud SQL)
- Local development environment setup steps
In order to get started, you must set up both a PostgreSQL instance and PGVector alongside it. + +Quick Start with Docker: +```yaml +version: '3.8' +services: + postgres: + image: ankane/pgvector:latest + environment: + POSTGRES_USER: fiftyone_user + POSTGRES_PASSWORD: your_password + POSTGRES_DB: fiftyone + ports: + - "5432:5432" +``` + +For production environments:
181-185: Add performance tuning guidelines for production use.Consider adding a section about:
- Recommended work_mem settings for different dataset sizes
- Index type selection based on use case
- Connection pooling configuration
export FIFTYONE_BRAIN_DEFAULT_SIMILARITY_BACKEND=pgvector + +# Performance Tuning Guidelines: +# For datasets < 1M vectors: work_mem = "64MB" +# For datasets < 10M vectors: work_mem = "128MB" +# For datasets > 10M vectors: work_mem = "256MB" + +# Connection Pooling: +export FIFTYONE_PGVECTOR_POOL_SIZE=10 +export FIFTYONE_PGVECTOR_POOL_TIMEOUT=30
268-279: Add batch operation examples for managing brain runs.Consider adding examples for:
- Batch deletion of multiple brain runs
- Filtering and managing runs by date
- Automated cleanup of old runs
# List all brain runs dataset.list_brain_runs() + +# Batch operations example: +# Delete all similarity runs older than 30 days +from datetime import datetime, timedelta +cutoff_date = datetime.now() - timedelta(days=30) +old_runs = [ + key for key in dataset.list_brain_runs(type=fob.Similarity) + if dataset.get_brain_info(key).timestamp < cutoff_date +] +for key in old_runs: + dataset.delete_brain_run(key)
617-619: Add performance benchmarks and metrics.Consider adding:
- Performance comparison between different metrics (cosine vs euclidean)
- Memory usage patterns for different dataset sizes
- Query latency benchmarks
The `metric` parameter may impact the quality of your query results, as well as the time and memory required to perform approximate nearest neighbor searches. Additionally, you can specify a parameter like `work_mem` to further tune performance. + +Performance Benchmarks: +| Metric | Dataset Size | Query Latency | Memory Usage | +|-----------|--------------|---------------|--------------| +| cosine | 1M vectors | ~10ms | 2GB | +| euclidean | 1M vectors | ~12ms | 2GB | +| dot | 1M vectors | ~8ms | 2GB |
|
@xenophenes should be good to go after the 2 suggestions. Not sure if the coderabit suggestion is good for the "dangling code" just take a look. Ping me when done |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
docs/source/integrations/pgvector.rst (3)
57-58: Add a concrete connection string example.The current connection string format is generic. Consider adding a practical example to help users understand the format better.
-export FIFTYONE_DATABASE_URI='postgresql://[userspec@][hostspec][/dbname][?paramspec]' +export FIFTYONE_DATABASE_URI='postgresql://[userspec@][hostspec][/dbname][?paramspec]' + +# Example: +export FIFTYONE_DATABASE_URI='postgresql://fiftyone_user:password123@localhost:5432/fiftyone_db?sslmode=require'
177-179: Enhance code example with required parameters.The current example is too minimal and doesn't show the required parameters for a working configuration.
import fiftyone.brain as fob -fob.compute_similarity(..., backend="pgvector", ...) +fob.compute_similarity( + dataset, + model="clip-vit-base32-torch", + brain_key="pgvector_index", + backend="pgvector", + embedding_column="embeddings" +)
500-501: Use more representative example data.Using random data for embeddings might not accurately represent real-world usage patterns and dimensionality requirements.
- embeddings = np.random.rand(20, 512) + # Generate embeddings using the same model for consistency + embeddings = model.embed_images([sample.filepath for sample in samples]) pgvector_index.add_to_index(embeddings, sample_ids)
| # Delete the PGVector index | ||
| pgvector_index = dataset.load_brain_results(brain_key) | ||
| pgvector_index.cleanup() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for cleanup operation.
The cleanup operation should include error handling to ensure resources are properly released, even if the operation fails.
# Delete the PGVector index
- pgvector_index = dataset.load_brain_results(brain_key)
- pgvector_index.cleanup()
+ try:
+ pgvector_index = dataset.load_brain_results(brain_key)
+ pgvector_index.cleanup()
+ except Exception as e:
+ print(f"Failed to cleanup PGVector index: {e}")
+ raise📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Delete the PGVector index | |
| pgvector_index = dataset.load_brain_results(brain_key) | |
| pgvector_index.cleanup() | |
| # Delete the PGVector index | |
| try: | |
| pgvector_index = dataset.load_brain_results(brain_key) | |
| pgvector_index.cleanup() | |
| except Exception as e: | |
| print(f"Failed to cleanup PGVector index: {e}") | |
| raise |
| Setup | ||
| _____ | ||
|
|
||
| In order to get started, you must set up both a PostgreSQL instance with the PGVector extension installed. |
There was a problem hiding this comment.
Fix grammatical error in setup instruction.
The sentence has redundant wording with "both".
-In order to get started, you must set up both a PostgreSQL instance with the PGVector extension installed.
+In order to get started, you must set up a PostgreSQL instance with the PGVector extension installed.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| In order to get started, you must set up both a PostgreSQL instance with the PGVector extension installed. | |
| In order to get started, you must set up a PostgreSQL instance with the PGVector extension installed. |
| pgvector_index = fob.compute_similarity( | ||
| dataset, | ||
| model="clip-vit-base32-torch", | ||
| embeddings_column="embeddings", # the column in which to store the embeddings | ||
| brain_key="pgvector_index", |
There was a problem hiding this comment.
Fix inconsistent parameter naming.
The parameter name embeddings_column is inconsistent with previous examples which use embedding_column.
fob.compute_similarity(
dataset,
model="clip-vit-base32-torch",
- embeddings_column="embeddings", # the column in which to store the embeddings
+ embedding_column="embeddings", # the column in which to store the embeddings
brain_key="pgvector_index",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pgvector_index = fob.compute_similarity( | |
| dataset, | |
| model="clip-vit-base32-torch", | |
| embeddings_column="embeddings", # the column in which to store the embeddings | |
| brain_key="pgvector_index", | |
| pgvector_index = fob.compute_similarity( | |
| dataset, | |
| model="clip-vit-base32-torch", | |
| embedding_column="embeddings", # the column in which to store the embeddings | |
| brain_key="pgvector_index", |
|
LGTM |
There was a problem hiding this comment.
@xenophenes amazing! Thanks so much for the contribution!! 🥇
@thesteve0 can you handle the comments I left on the PR?
Also, there's a couple additional places to mention new integrations. Can you add those?
The recent Elasticsearch vector search integration shows where the additions are called for: https://github.com/voxel51/fiftyone/pull/4431/files
| .. pgvector-integration: | ||
|
|
||
| PostgreSQL Vector Search Integration | ||
| ================================= |
There was a problem hiding this comment.
nit: Sphinx emits errors if the === don't exactly match the length of the text above them
|
|
||
| .. default-role:: code | ||
|
|
||
| `PostgreSQL <https://www.postgresql.org>`_ is the world's most advanced open source database, and we've made it easy to use PostgreSQL's `vector search capabilities using PGVector <https://github.com/pgvector/pgvector>`_ on your computer vision data directly from FiftyOne! |
There was a problem hiding this comment.
nit: our docs style guide requests that maximum line length is 79 characters.
Unfortunately, enforcing this via pre-commit hooks causes bad things to happen (eg tables are broken, I believe), so the line length must be manually enforced via your ENTER key...
| dataset.set_values("embeddings", embeddings.tolist()) | ||
| fob.compute_similarity( | ||
| dataset, | ||
| embedding_column="embeddings", # the field that contains the embeddings |
There was a problem hiding this comment.
Does embedding_column really specify both the PGvector column to use and a pre-existing FiftyOne field from which to load pre-computed embeddings?
The fob.compute_similarity() interface already mandates that the embeddings parameter should be usable for all backends to load pre-existing embeddings and/or persist embeddings that are computed by the backend.
So the syntax I would expect, albeit a bit redundant, would be:
fob.compute_similarity(
dataset,
embeddings="embeddings", # the FO field that contains the embeddings
embedding_column="embeddings", # the PGvector column in which to store the embeddings
backend="pgvector",
brain_key=brain_key,
)
There was a problem hiding this comment.
@brimoor Yes - will do after the engineering code review and test suite PR
What changes are proposed in this pull request?
Related to the PGVector integration, and this PR in fiftyone-brain: voxel51/fiftyone-brain#222
How is this patch tested? If it is not, please explain why.
It is.
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)
Addition of documentation describing PGVector integrations as per PR to fiftyone-brain: voxel51/fiftyone-brain#222
What areas of FiftyOne does this PR affect?
fiftyonePython library changesSummary by CodeRabbit