feat(core): add interval-safe exact top-k - #58
Conversation
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
42bca16
into
docs/product-technical-gap-baseline
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| absolute_normalized_vectors | ||
| .extend(normalized_vectors[offset..].iter().map(|value| value.abs())); |
There was a problem hiding this comment.
🔴 Non-macOS indexes double vector memory
On non-macOS hosts, absolute_normalized_vectors retains a second full matrix that no available backend can read. Existing large indexes can exhaust memory without invoking top-k.
Prompt for agents
Avoid retaining the absolute normalized matrix on platforms where the Accelerate implementation is not compiled. In crates/rankweave-core/src/semantic_index.rs, make the field and its build-time allocation, population, and initialization conditional on macOS, or construct equivalent metadata lazily only when the macOS top-k backend needs it. Preserve the existing normalized matrix and scalar behavior on Linux and Windows.
Was this helpful? React with 👍 or 👎 to provide feedback.
| item_intervals | ||
| .entry(item_id) | ||
| .and_modify(|current| { | ||
| current.0 = current.0.max(interval.0); | ||
| current.1 = current.1.max(interval.1); | ||
| }) | ||
| .or_insert(interval); | ||
| } | ||
| let ambiguity = ambiguous_items(&item_intervals, top_k); |
There was a problem hiding this comment.
📝 Info: Screening occurs after item pooling
ambiguous_items retains boundary equality after unit intervals pool by item. Scalar recomputation then covers every unit of each retained item.
Was this helpful? React with 👍 or 👎 to provide feedback.
| self.rank_authorized_batch_refs(model_identity, query_vectors, authorized_candidate_ids) | ||
| .map(|reports| { | ||
| reports | ||
| .into_iter() | ||
| .zip(query_vectors) | ||
| .map(|(report, query)| { | ||
| self.finish_top_k_report( | ||
| &model_digest, | ||
| query, | ||
| authorized_candidate_ids, | ||
| report.results, | ||
| top_k, | ||
| SEMANTIC_INDEX_TOP_K_CPU_EXECUTION_PROFILE, | ||
| ) | ||
| }) | ||
| .collect() | ||
| }) |
There was a problem hiding this comment.
📝 Info: Fallback keeps separate top-k evidence
scalar_top_k_batch_refs rebuilds both digests through finish_top_k_report. Fallback reports therefore bind top_k and never reuse full-ranking evidence.
Was this helpful? React with 👍 or 👎 to provide feedback.
| #[cfg(target_os = "macos")] | ||
| fn rank_authorized_top_k_accelerate_refs( | ||
| &self, | ||
| model_identity: &str, | ||
| query_vectors: &[&[f64]], | ||
| authorized_candidate_ids: &[(&str, &str)], | ||
| top_k: usize, | ||
| ) -> Result<Vec<SemanticIndexRankingReport>, SemanticIndexError> { | ||
| let model_digest = digest_bytes( | ||
| b"rankweave.semantic-unit-index.model.v1\0", | ||
| [model_identity.as_bytes()], | ||
| ); | ||
| if model_digest != self.evidence.model_digest { | ||
| return Err(SemanticIndexError::ModelMismatch); | ||
| } | ||
| if query_vectors.is_empty() { | ||
| return Err(SemanticIndexError::EmptyQueryBatch); | ||
| } | ||
| if authorized_candidate_ids.is_empty() { | ||
| return Err(SemanticIndexError::EmptyAuthorization); | ||
| } | ||
| let mut authorization_seen = HashSet::new(); | ||
| let mut authorized_indices = Vec::with_capacity(authorized_candidate_ids.len()); | ||
| for (item_id, unit_id) in authorized_candidate_ids { | ||
| if !authorization_seen.insert((item_id, unit_id)) { | ||
| return Err(SemanticIndexError::DuplicateAuthorization { | ||
| item_id: (*item_id).to_owned(), | ||
| unit_id: (*unit_id).to_owned(), | ||
| }); | ||
| } | ||
| let Some(index) = self | ||
| .candidate_lookup | ||
| .get(*item_id) | ||
| .and_then(|units| units.get(*unit_id)) | ||
| else { | ||
| return Err(SemanticIndexError::UnknownAuthorizedCandidate { | ||
| item_id: (*item_id).to_owned(), | ||
| unit_id: (*unit_id).to_owned(), | ||
| }); | ||
| }; | ||
| authorized_indices.push(*index); | ||
| } | ||
| let Some(roundoff) = DotRoundoffBound::new(self.evidence.vector_dimension) else { | ||
| return self.scalar_top_k_batch_refs( | ||
| model_identity, | ||
| query_vectors, | ||
| authorized_candidate_ids, | ||
| top_k, | ||
| ); | ||
| }; | ||
| let prepared_queries = query_vectors | ||
| .iter() | ||
| .map(|query| self.prepare_query(query)) | ||
| .collect::<Result<Vec<_>, _>>()?; | ||
| let Some((approximate_dots, approximate_absolute_dots)) = accelerate_matrix_multiply_pair( | ||
| &self.normalized_vectors, | ||
| &self.absolute_normalized_vectors, | ||
| &prepared_queries, | ||
| self.evidence.candidate_count, | ||
| self.evidence.vector_dimension, | ||
| ) else { | ||
| return self.scalar_top_k_batch_refs( | ||
| model_identity, | ||
| query_vectors, | ||
| authorized_candidate_ids, | ||
| top_k, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
📝 Info: Platform paths share failure semantics
The accelerated path validates all inputs before GEMM, while every fallback reuses scalar validation. Zero top_k also fails before platform dispatch.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Evidence
Activation boundary
This is a stacked prerequisite on #41. It is insufficient by itself to prove the LineageWeave 20 ms read contract: a 500-iteration full-path phase trace still observed intermittent owner/host scheduling tails, including a 61.509 ms owner phase. No downstream activation or threshold change is included.
Signed-off-by: Seongho Bae seonghobae@users.noreply.github.com