Skip to content

fix: address embedding correctness and worker configuration - #766

Closed
RAMZI0TO99 wants to merge 1 commit into
qdrant:mainfrom
RAMZI0TO99:fix/combined-contributions
Closed

RAMZI0TO99 wants to merge 1 commit into
qdrant:mainfrom
RAMZI0TO99:fix/combined-contributions

Conversation

@RAMZI0TO99

Copy link
Copy Markdown
Contributor

This consolidates six fixes for sparse embedding edge cases, parallel worker configuration, and ConvNeXT preprocessing. Each fix includes regression tests and docstrings for the affected functions.

Reproduction or trigger Before After
SparseEmbedding.from_dict({}) used for NumPy indexing Empty indices default to float64 and cause IndexError Empty indices are int64; indexing and concatenation work
BM25 query token ad1u66pi Its signed hash is -2147483648; absolute ID 2147483648 cannot fit in int32 Query indices use int64 and match document IDs
Parallel pool with cuda=False and no device IDs Workers do not receive the CPU selection Every worker receives the pool's cuda setting; device assignment stays round-robin
ConvNeXT with do_resize=False A 96x160 image becomes 224x224; pixels can change even at the target size Resize and its associated crop are skipped; rescaling and normalization can still run
SPLADE float32 logit 1e-8 log(1+x) rounds its weight to zero and drops the dimension log1p(x) preserves the small positive weight
BM25 bag kitchens kitchens prostaglandins, reordered The shared ID's weight changes from 1.6786885245901642 to 1.904311073541843 Proposed accumulation gives 3.582999598132007 for either ordering

The hash function and existing token IDs are preserved. The resize flag still defaults to enabled when omitted. SPLADE masking, maximum pooling, and nonzero filtering are unchanged.

This is a draft for collision-policy review. Related: #369. kitchens and prostaglandins have signed hashes 358434922 and -358434922, so their absolute IDs collide. This proposal sums their separately calculated lexical BM25 weights. Aggregating their counts before applying the nonlinear BM25 formula is another policy and would produce 1.9936283185840709 for the example. The collision regressions check order independence within floating-point tolerance without prescribing either policy. Maintainer input is requested on which policy belongs here.

Corrected collision weights require recomputing affected stored document vectors for consistent use. Preserving IDs does not eliminate hash aliases. No retrieval-quality improvement or bitwise order invariance is claimed.

Validation against the actual checkout, FastEmbed 0.8.1 on Windows 11 build 10.0.26200, Python 3.13.5, NumPy 2.3.5, and ONNX Runtime 1.30.0 CPU:

  • 115 selected cases passed: all six new regression modules plus existing common, parallel-processor, and image-transform tests.
  • Six existing SPLADE model checks passed with actual ONNX revision 600ce23fd7611e71694fcf724f46778f1ad24c1c, covering batch/single reference embeddings, parallel processing, lazy loading, session options, and token counts. Canonical expectations were unchanged. The single-embedding test used its existing model-selection hook to select SPLADE.
  • The 18 new SPLADE cases also passed on Python 3.10.11 / NumPy 2.2.6.
  • Ruff 0.3.4 lint and formatting, staged whitespace, and patch checks passed.
  • Repository type-check commands passed on Python 3.13.5: mypy 2.4.0 reported no issues in 65 source files; pyright 1.1.414 reported zero errors or warnings.
  • Local AST docstring coverage is 50/50 touched functions (100%, above the reported 80% CodeRabbit threshold). Six new modules and the new worker class are also documented. This is a local audit; CodeRabbit's own result will be reported separately.
  • The final documentation update preserved executable ASTs in all 11 files, so the recorded functional results still apply. Tests were not rerun solely for those docstrings.

The full model suite, GPU execution, retrieval benchmarks, and full CI matrix were not run locally. The model-test runner emitted one non-failing anyio assertion-rewrite warning.

This combined submission supersedes #762, #763, #764, and #765. The UTF-8 stopword candidate is excluded following duplicate review because #684 already contains that production change.

@joein

joein commented Oct 3, 2026

Copy link
Copy Markdown
Member

Hey @RAMZI0TO99
thanks for pointing out the bugs in corner cases
Though, we can't merge the pr as is, could you create a PR per problem?

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.

2 participants