ITEP-90240: Reid vector length is configurable#1301
Conversation
There was a problem hiding this comment.
Pull request overview
Makes ReID embedding vector length configurable so different ReID models (with different embedding dimensions) can be supported across the pipeline, controller, and VDMS storage.
Changes:
- Allow variable-length ReID embeddings to be packed in the DLStreamer gvapython policy.
- Parameterize VDMS vector dimension checks (and schema dimensions) via a
dimensionsfield onVDMSDatabase. - Add
vector_dimensionstocontroller/config/reid-config.jsonand plumb it intoUUIDManagerso the controller can configure VDMS dimensions.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| dlstreamer-pipeline-server/user_scripts/gvapython/sscape/sscape_policies.py | Packs ReID embeddings using their runtime length instead of a hard-coded 256. |
| controller/src/controller/vdms_adapter.py | Uses self.dimensions when validating vectors before storing into VDMS. |
| controller/src/controller/uuid_manager.py | Reads vector_dimensions from ReID config and passes it into the VDMS adapter. |
| controller/config/reid-config.json | Adds the new vector_dimensions configuration field. |
| # Handle variable-length re-id vectors from different models | ||
| if not reid_vector: | ||
| continue | ||
| vector_len = len(reid_vector) | ||
| # Pack vector with its actual dimensions | ||
| format_string = f"{vector_len}f" | ||
| try: | ||
| v = struct.pack(format_string, *reid_vector) |
There was a problem hiding this comment.
The pipeline now packs the ReID embedding using its runtime length, but the Scene Controller currently decodes base64 embeddings with a fixed struct.unpack("256f", ...) (e.g., controller/src/controller/moving_object.py). With vector_dimensions configurable on the controller side, this change will make non-256 embeddings fail to decode or be silently dropped later due to dimension checks. Consider emitting the vector length/dimensions alongside the embedding (or keeping the packing size aligned to the configured dimensions) so producer/consumer stay compatible.
| for reid_vector in reid_vectors: | ||
| # Ensure vector is properly formatted as 1D array of float32 | ||
| # reid_vector might be shape (1, 256) from moving_object, need to flatten to (256,) | ||
| # reid_vector might be shape (N,) from moving_object, need to flatten to (N,) | ||
| vec_array = np.asarray(reid_vector, dtype="float32").flatten() | ||
| if vec_array.shape[0] != 256: | ||
| log.warning(f"addEntry: Expected vector shape (256,) but got {vec_array.shape}, skipping this vector") | ||
| if vec_array.shape[0] != self.dimensions: | ||
| log.warning(f"addEntry: Expected vector shape ({self.dimensions},) but got {vec_array.shape}, skipping this vector") | ||
| continue |
There was a problem hiding this comment.
If all vectors are skipped due to a dimension mismatch, add_query and descriptor_blobs can be empty but sendQuery(add_query, descriptor_blobs) is still called. This can produce misleading errors ("No response" for 0 vectors) or potentially trigger errors in the VDMS client when querying with an empty command list. Consider short-circuiting when no valid vectors remain (log at debug/warn and return) before calling sendQuery.
| # Extract vector dimensions from reid config, pass to database | ||
| if reid_config_data is None: | ||
| reid_config_data = {} | ||
| vector_dimensions = reid_config_data.get('vector_dimensions', DIMENSIONS) | ||
| self.reid_database = available_databases[database](dimensions=vector_dimensions) | ||
| self.pool = concurrent.futures.ThreadPoolExecutor() |
There was a problem hiding this comment.
vector_dimensions is now read from reid_config_data and passed into VDMSDatabase(dimensions=...), but the existing UUIDManager unit tests don’t verify that this configuration value is actually applied. Adding a test that initializes UUIDManager(reid_config_data={"vector_dimensions": <non-default>}) and asserts VDMSDatabase is constructed with that dimensions argument would prevent regressions.
| "feature_slice_size": 10, | ||
| "similarity_threshold": 60 | ||
| "similarity_threshold": 60, | ||
| "vector_dimensions": 256 | ||
| } |
There was a problem hiding this comment.
A new vector_dimensions option is added to reid-config.json, but the controller documentation that describes this config file (e.g. docs/user-guide/microservices/controller/Extended-ReID.md) does not mention this setting yet. Please document what this value controls (VDMS schema dimensions + expected embedding length) and any compatibility constraints with the embedding producer/decoder.
📝 Description
Provide a clear summary of the changes and the context behind them. Describe what was changed, why it was needed, and how the changes address the issue or add value.
✨ Type of Change
Select the type of change your PR introduces:
🧪 Testing Scenarios
Describe how the changes were tested and how reviewers can test them too:
✅ Checklist
Before submitting the PR, ensure the following: