Skip to content

Commit fb9139b

Browse files
committed
docs(pipnn): clarify invariants flagged in review
- Note that the IP branch of distance_from_dot must return before the norm lookups because norms_ is never allocated for IP. - Document that group_duplicate_rows deduplicates bitwise on raw row bytes, so -0.0f and +0.0f are intentionally distinct. - Document that odescent_param_->max_degree is mutated in place by the bottom graph build, which is pre-existing 1.0 behaviour. No behaviour change. Signed-off-by: 庄叶 <zhuangye.yxw@antgroup.com>
1 parent 2683f3c commit fb9139b

2 files changed

Lines changed: 12 additions & 0 deletions

File tree

‎src/algorithm/hgraph/hgraph_build.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,11 @@ HGraph::build_by_batch_graph(const DatasetPtr& data, bool use_pipnn) {
345345
}
346346
}
347347
} else {
348+
// Note: odescent_param_ is a shared member and this mutates it in place, so
349+
// max_degree is left at the bottom-graph degree after this call. That is
350+
// pre-existing 1.0 behaviour (the route-graph loop below has always
351+
// overwritten the same shared field), and the PiPNN route-graph path copies
352+
// the parameter before adjusting it for exactly this reason.
348353
odescent_param_->max_degree = bottom_graph_->MaximumDegree();
349354
ODescent odescent_builder(
350355
odescent_param_, build_data, allocator_, this->thread_pool_.get());

‎src/impl/pipnn/pipnn_graph_builder.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,10 @@ group_duplicate_rows(const Vector<InnerIdType>& ids,
156156
const uint64_t row_bytes = checked_product(dimensions, sizeof(float), "row");
157157
require_argument(row_bytes <= std::numeric_limits<std::size_t>::max(),
158158
"PiPNN row size exceeds the platform limit");
159+
// Deduplication is intentionally bitwise: the key is a string_view over the raw
160+
// float bytes, so two rows compare equal only when every byte matches. This is what
161+
// PiPNN wants (a duplicate vector is one that is identical on disk), and it means
162+
// -0.0f and +0.0f are treated as distinct rows rather than being merged.
159163
UnorderedMap<std::string_view, InnerIdType> representatives(allocator);
160164
representatives.reserve(rows.size());
161165

@@ -1203,8 +1207,11 @@ PiPNNPipeline::distance_from_dot(uint32_t lhs, uint32_t rhs, float dot) const {
12031207
return std::max(0.0F, sanitize_distance(sum - dot - dot));
12041208
}
12051209
if (metric_ == MetricType::METRIC_TYPE_IP) {
1210+
// norms_ is never allocated for IP (see prepare()), so this branch must
1211+
// return before the norm lookups below.
12061212
return sanitize_distance(1.0F - dot);
12071213
}
1214+
// Cosine (and any other norm-normalized metric).
12081215
const float similarity = std::clamp(dot * norms_[lhs] * norms_[rhs], -1.0F, 1.0F);
12091216
return sanitize_distance(1.0F - similarity);
12101217
}

0 commit comments

Comments
 (0)