Test merge virtual people core serving - #4278
Draft
laureanobrs wants to merge 198 commits into
Draft
Conversation
…thors Creates AUTHORS file for the repository.
https://arxiv.org/pdf/1406.2294.pdf. The impelmentation is copied from Figure 1 on page 2 in https://arxiv.org/pdf/1406.2294.pdf.
…nsistent-hashing Create consistent hashing library based on a published paper
implementation of CompiledNode proto.
…ed on the consistent hashing.
Only PopulationNodeImpl will be created in the factory currently.
…pulation-node Build population node
The details of the algorithm and the original Python implementation can be found in https://github.com/world-federation-of-advertisers/virtual_people_examples/blob/main/notebooks/Consistent_Hashing.ipynb.
Update comments and test names. Replace "for" with "for_each". Replace "std::pair" with struct for distribution choice. Replace "std::string" with "absl::string_view". Replace "log" from math.h with "std::log". Use macro to handle "absl::StatusOr" checking and assigning.
* Add LruCache implementation * Add LruCache implementation * Update LruCache implementation to use std::tm as key add initial version of GetEventId function * Fix LruCache compilation errors * Add missing methods in VidModelSelector class * Remove unusued variable * Add missing getModelRelease function * Add logic to validate model_line and model_rollouts inside the constructor * Add test for VidModelSelector C++ * typos * run linter * Add fix for broken linting * Add comments moved common textproto files into a common directory removed unapproved regex header * Moved proto files rename variable in LRU cache * Add hash / comparison functions to LruCache Add Todo Move model_line_cc_proto and model_rollout_cc_proto definition into a different package * Move comments into header files * Remove exceptions in favor of absl::status * Run linter * Replace std::time with absl::CivilDay * Make VidModelSelector as private * Run linter * Add const modifier Make utility function to a non-member function * Mark assignment operator as deleted * Run linter * Remove test/proto folder fix emplace_back function usage Remove redundant cc_proto_library definition * Run linter * Add constructor to ModelReleasePercentile struct * Remove unusued code * Fix LruCache to remove least recently used element instead of oldest key * Run linter * remove unusued import --------- Co-authored-by: marcopremier <marcopremier@meta.com>
* Migrate to Bzlmod
* expose vid labeler resources for use in other projects as testonly resources
* Add memoized VID assignment (Kotlin) - Feistel cipher utility: format-preserving bijection for collision-free VID assignment, cross-language deterministic with C++ implementation - RankedPopulationNodeImpl: ranked (Feistel) + unranked (hash) VID assignment with DISJOINT and FULL_POOL modes - Wire RankedPopulationNode into ModelNode factory - Add LabelingMode enum and pass-1 (pool-identity) mode to Labeler * Add Kotlin tests for memoized VID assignment - FeistelTest: bijectivity, determinism, range, edge cases, prime domains - RankedLabelingIntegrationTest: two-pass collision-free, mixed model, ranked_size=0 fallback * Fix: add RankedPopulationNodeImpl.kt to model_node BUILD srcs * Update Kotlin impl to match final merged protos - Use nested RankedPopulationNode.UnrankedMode (was top-level) - Remove pass-1 mode (LabelingMode, pool_identity_mode) — will be added in a follow-up once LabelerEvent proto field is added - Revert Labeler to original label(input) signature - Rewrite integration tests to test ranked/unranked VID assignment directly without pass-1 mode * Address Steven's review feedback on Kotlin implementation - Make Feistel cycle-walk iterative instead of recursive - Add golden test vectors for cross-language C++ parity - Use exhaustive when for UnrankedMode - Use jumpConsistentHash instead of seed % poolSize - Extract collapseQuantumLabel and computeVidSeed to shared PopulationNodeHelper utility - Use single() for pool access (only one pool supported) - Restructure Apply to use val-only (no var reassignment) - Add rank overflow test (local_rank >= ranked_size) * Fix iterative Feistel to match recursive output for C++ parity * Revert to recursive Feistel for C++ parity, add depth bound comment * Fix golden vectors test — pin outputs with TODO for C++ verification * Bump virtual-people-common to v0.6.0 and fix lint - Update MODULE.bazel dep from 0.4.0 to 0.6.0 (includes merged RankedPopulationNode and PoolAssignment protos) - Fix ktfmt formatting (when branch wrapping, KDoc param indent) * Fix lint formatting and upgrade rules_cc to 0.1.1 - Apply ktfmt formatting (line wrapping, single-line lambdas) - Upgrade rules_cc from 0.0.9 to 0.1.1 (0.1.0 yanked from BCR) * Address remaining review feedback - Make Feistel iterative (do/while cycle-walk) - Remove C++ parity comments (no C++ impl yet) - require(poolsCount == 1) with message - Update PopulationNodeImpl to use PopulationNodeHelper - Golden vectors test: print values for pinning (temporary) * Pin golden vectors with verified values from iterative Feistel * Add unit tests for PopulationNodeHelper * Address remaining review feedback - Revert rules_cc bump (scope to C++ PR, not this one) - Replace ranked_size=0 range test with VID equality test against PopulationNode — proves drop-in replacement compatibility * Restore rules_cc 0.1.1 — required by virtual-people-common v0.6.0 transitive dep * Re-pin maven_install.json for virtual-people-common v0.6.0 * Add comment explaining rules_cc 0.1.1 override * Fix remaining ktfmt lint issues * Bump virtual-people-common to v0.6.1 (includes proto changes and rules_cc fix) * build: Re-pin maven_install.json Regenerate lockfile to match MODULE.bazel input signature. * Add tests for boundary rank and multiple rank assignments Per Raimundo's review: - Boundary: local_rank == ranked_size falls back to unranked (off-by-one check) - Multiple rank assignments: event with 3 RankAssignment entries resolves correct pool by pool_offset matching
* refactor: optimize Kotlin Labeler hot path * refactor: drop unused optimizations * fix: remove unoptimized code from the PR * test: add comparison tests for hasing functions * refactor: rever builder to dsl * refactor: based on PR comments * refactor: based on PR comments * test: add test for new debug proto field * refactor: updated virtual-people-common * refactor: based on PR comments * refactor: resolved conflicts * docs: run linter
* Add pass-1 (pool-identity) labeling mode (Kotlin)
- LabelingMode enum (FULL, POOL_IDENTITY) on Labeler
- RankedPopulationNodeImpl checks pool_identity_mode and emits
PoolAssignment instead of assigning a VID
- Three new tests: pass-1 emits pool assignment, PopulationNode
unaffected, full two-pass collision-free end-to-end
* Address Steven's review feedback on pass-1 mode
- PopulationNodeImpl throws IllegalStateException if pool_identity_mode
is true (Option 2 — explicit failure for unsupported node type)
- Fix confusing .let assertion to plain assertEquals
- Add test: rank for non-existent pool falls back to unranked
- Update PopulationNode test to expect IllegalStateException
* Fix import ordering for ktfmt
* Fix ktfmt: single-line assertFailsWith, lambda indentation
* Fix ktfmt: split input.toBuilder() chain
* Address review feedback on pass-1 mode
- Remove unnecessary poolAssignments count guard
- Use Kotlin DSL (+=) instead of addAll
- Throw on mismatched RankAssignment (caller misuse, not silent fallback)
- Add boundary test: local_rank == ranked_size
- Add pass-1 branch routing test: two pools, verify correct routing
- Add determinism test: two-pass run twice yields same VIDs
- Update non-existent pool test to expect exception
- Use Kotlin DSLs throughout tests
* Fix mergeFrom — use toBuilder() pattern for proto mutation
* Address remaining review comments
- Use Kotlin DSL for PoolAssignment in RankedPopulationNodeImpl
- Restore multiple rank assignments in test (100, 500, 9000)
- Use input.copy {} instead of toBuilder() pattern
* Fix: use toBuilder() — copy {} not available for LabelerInput
* Fix import order for ktfmt
* Fix ktfmt: wrap DSL arguments on separate lines
* Use copy {} in determinism test, bump virtual-people-common to v0.7.0
- Last nit: replace toBuilder() with input.copy {} in determinism test
- Bump dep to v0.7.0 (includes pool_identity_mode proto)
* Fold back pool assignments for multiplicity events in pass-1 mode
applyMultiplicity folded back only virtualPersonActivitiesList from clones,
so pool assignments emitted in pool-identity (pass-1) mode were dropped for
multiplicity events. Also fold back poolAssignmentsList. Adds a regression
test; ktfmt reflows the now-touched file KDoc.
* PopulationNode no-ops in pass-1 instead of throwing
Per review: a model may mix RankedPopulationNode and PopulationNode leaves.
In pool-identity (pass-1) mode a plain PopulationNode now returns without
emitting a PoolAssignment or VID; pass-2 labels those events normally. Updates
the pass-1 PopulationNode test to verify the no-op and adds a mixed-model
integration test (ranked + vanilla leaves under one BranchNode).
---------
Co-authored-by: yijuilee <yijuilee@users.noreply.github.com>
* Add Feistel cipher utility for collision-free VID assignment Format-preserving bijection mapping [0, N) -> [0, N) with zero collisions. Uses a 4-round Feistel network with FarmHash64 as the round function and cycle-walking for non-power-of-2 domains. Tests cover bijectivity, determinism, range bounds, edge cases (domain 0, 1, 2), and prime-sized domains. * Make C++ Feistel iterative, add cross-language parity test - Iterative cycle-walking (matches Kotlin, avoids deep recursion) - Golden vector test verifies identical output to Kotlin Feistel - Bump virtual-people-common to v0.7.0 --------- Co-authored-by: yijuilee <yijuilee@users.noreply.github.com>
* Add Feistel cipher utility for collision-free VID assignment Format-preserving bijection mapping [0, N) -> [0, N) with zero collisions. Uses a 4-round Feistel network with FarmHash64 as the round function and cycle-walking for non-power-of-2 domains. Tests cover bijectivity, determinism, range bounds, edge cases (domain 0, 1, 2), and prime-sized domains. * Add RankedPopulationNodeImpl for memoized VID assignment (C++) New leaf node type that splits VID assignment into: - Ranked path: Feistel bijection for collision-free VID assignment when a pre-computed rank is available in LabelerInput.rank_assignments - Unranked path: hash-based fallback with DISJOINT (separate pool) or FULL_POOL (shared pool) modes Also registers the new node type in ModelNode::Build dispatch. * Fix: make RankedPopulationNodeImpl constructor public * Make C++ Feistel iterative, add cross-language parity test - Iterative cycle-walking (matches Kotlin, avoids deep recursion) - Golden vector test verifies identical output to Kotlin Feistel - Bump virtual-people-common to v0.7.0 * Update RankedPopulationNodeImpl (C++) to final design - Use nested RankedPopulationNode::UnrankedMode (proto change) - Require exactly one pool (matches Kotlin single()) - Use JumpConsistentHash for unranked path (matches PopulationNode) - Throw on mismatched RankAssignment (caller misuse, not silent fallback) - Add boundary, multiple-rank, non-existent-pool, multi-pool tests * Apply clang-format (Google style) * Add missing include for std::move (cpplint) * Address review on RankedPopulationNode (C++) - Guard pool_size against INT32_MAX before JumpConsistentHash so a pool larger than the design max fails loudly instead of silently wrapping to a wrong VID. - Extract CollapseQuantumLabel into a shared population_node_helper used by both PopulationNodeImpl and RankedPopulationNodeImpl, removing the duplicated copy. - Add a backward-compat test that RankedPopulationNode(ranked_size=0, FULL_POOL) assigns identical VIDs to PopulationNode for the same pool and seed. --------- Co-authored-by: yijuilee <yijuilee@users.noreply.github.com>
* Add Feistel cipher utility for collision-free VID assignment Format-preserving bijection mapping [0, N) -> [0, N) with zero collisions. Uses a 4-round Feistel network with FarmHash64 as the round function and cycle-walking for non-power-of-2 domains. Tests cover bijectivity, determinism, range bounds, edge cases (domain 0, 1, 2), and prime-sized domains. * Add RankedPopulationNodeImpl for memoized VID assignment (C++) New leaf node type that splits VID assignment into: - Ranked path: Feistel bijection for collision-free VID assignment when a pre-computed rank is available in LabelerInput.rank_assignments - Unranked path: hash-based fallback with DISJOINT (separate pool) or FULL_POOL (shared pool) modes Also registers the new node type in ModelNode::Build dispatch. * Fix: make RankedPopulationNodeImpl constructor public * Add pass-1 (pool-identity) labeling mode In kPoolIdentity mode, RankedPopulationNode emits PoolAssignment (pool_offset, pool_size, ranked_size) instead of assigning a VID. PopulationNode is unaffected (backward-compatible). This enables the two-pass memoized VID assignment pipeline. * Add end-to-end integration test for memoized VID assignment Full two-pass test: build model with RankedPopulationNode leaves, run pass-1 (pool identity), simulate ranking, run pass-2 (VID assignment), verify collision-free VIDs in correct ranges. Also tests mixed model (PopulationNode + RankedPopulationNode) and ranked_size=0 fallback behavior. * Make C++ Feistel iterative, add cross-language parity test - Iterative cycle-walking (matches Kotlin, avoids deep recursion) - Golden vector test verifies identical output to Kotlin Feistel - Bump virtual-people-common to v0.7.0 * Update RankedPopulationNodeImpl (C++) to final design - Use nested RankedPopulationNode::UnrankedMode (proto change) - Require exactly one pool (matches Kotlin single()) - Use JumpConsistentHash for unranked path (matches PopulationNode) - Throw on mismatched RankAssignment (caller misuse, not silent fallback) - Add boundary, multiple-rank, non-existent-pool, multi-pool tests * Fix: use count() instead of contains() (C++17) * Apply clang-format (Google style) * Apply clang-format (Google style) * Add missing includes for std::move and std::min (cpplint) * Add missing include for std::move (cpplint) * Fold back pool assignments for multiplicity events in pass-1 mode (C++) ApplyMultiplicity folded back only virtual_person_activities from clones, so pool assignments emitted in pool-identity (pass-1) mode were dropped for multiplicity events. Also fold back pool_assignments. Adds a regression test and the system includes cpplint now requires on the touched test file. * PopulationNode no-ops in pass-1 instead of erroring (C++) Per review: a model may mix RankedPopulationNode and PopulationNode leaves. In pool-identity (pass-1) mode a plain PopulationNode now returns OK with no output; pass-2 labels those events normally. Updates the pass-1 PopulationNode test to verify the no-op and adds a mixed-model integration test (ranked + vanilla leaves under one BranchNode). Mirrors the Kotlin change in #87. * Address review on RankedPopulationNode (C++) - Guard pool_size against INT32_MAX before JumpConsistentHash so a pool larger than the design max fails loudly instead of silently wrapping to a wrong VID. - Extract CollapseQuantumLabel into a shared population_node_helper used by both PopulationNodeImpl and RankedPopulationNodeImpl, removing the duplicated copy. - Add a backward-compat test that RankedPopulationNode(ranked_size=0, FULL_POOL) assigns identical VIDs to PopulationNode for the same pool and seed. * Add cross-language golden-vector tests for RankedPopulationNode Pins specific VIDs for the full ranked path, unranked DISJOINT, and unranked FULL_POOL paths, plus the pass-1 PoolAssignment, for a fixed config and inputs. The Feistel golden vectors only cover the ranked permutation; these additionally pin the unranked path (FarmHash64(random_seed + acting_fingerprint) -> JumpConsistentHash). A matching Kotlin golden test (separate PR) reproduces the same values so any cross-language divergence fails loudly. --------- Co-authored-by: yijuilee <yijuilee@users.noreply.github.com>
* build: Bump common-jvm, common-cpp, cross-media-measurement-api Bumps common-jvm 0.96.0 -> 0.144.0, common-cpp 0.13.0 -> 0.14.0, and cross-media-measurement-api 0.70.0 -> 0.94.0 to drop the transitive reference to the yanked googleapis@0.0.0-bzlmod.1. The new versions align with what cross-media-measurement currently uses. Switches labeler.cc to TextFormat::PrintToString for the debug trace. Newer protobuf intentionally makes DebugString() emit a redaction marker that is not parseable, which broke the existing round-trip test. Issue: None
…ignments (#89) * fix: RankedPopulationNode falls back to hash on non-matching rank_assignments A fingerprint can legitimately route to multiple subpools across impressions (design § Cumulative rank index map), so Phase-2 attaches every per-subpool rank the fingerprint holds. The leaf takes its own pool_offset; if none matches, the leaf must fall back to the hash path — same as when no rank assignments are provided at all. Previously the leaf threw InvalidArgument, failing the whole VidLabelingJob whenever a fingerprint reached a subpool it had no rank in (overflowed in Phase 1, or only seen in other subpools previously). The proto KDoc on LabelerInput.RankAssignment already specifies this fallback behavior; the code was the outlier. Tests updated in both languages: the prior tests that codified the throw (C++ RankForNonExistentPoolReturnsError, Kotlin `rank for non-existent pool throws`) are replaced with assertions that the VID lands in the unranked sub-range, plus a parity test that non-matching rank_assignments produce the same VID as empty rank_assignments. Issue: #4073 * docs: clarify overflow as the primary fallback case in KDocs and tests The earlier framing leaned on multi-subpool routing across impressions, but that phrasing implied an impression has multiple destinations when it does not. The deterministic labeler routes each impression to exactly one subpool leaf. The rank_assignments list can hold multiple entries only because the sink conservatively attaches every subpool the fingerprint is currently ranked in (across this DataProvider/ModelLine); the leaf then picks its own. The genuine, designed-for case where no entry matches is Phase-1 overflow: when this subpool reached ranked_size and this fingerprint was one of the unranked surplus, the subpool has no rank for it. The design's overflow-fps-fall-back-to-unranked-path contract requires the hash fallback this PR provides. * feat: set memoized_rank_fallback when ranked path is bypassed Stamps VirtualPersonActivity.memoized_rank_fallback (new in virtual-people-common#75) when the caller attempted memoization (LabelerInput.rank_assignments non-empty) but the leaf had to fall back to the hash path. The dominant cause is Phase-1 overflow: the subpool reached ranked_size and this fingerprint was one of the unranked surplus. Without this signal, the memoized Phase-2 pipeline silently degrades to hash-based VID assignment when subpools saturate. The consumer (VidLabelingSink in cross-media-measurement#4072) aggregates this per (data provider, model line, pool offset) into an OpenTelemetry counter to alarm on rising fallback rates — the indicator that a subpool's ranked_size should be raised. The field stays false on two cases that share the hash path: - No rank_assignments provided (caller did not attempt memoization). - Matching rank_assignments with local_rank in range (ranked path taken). Tests in both languages assert the field across all four cases (ranked, no rank_assignments, non-matching rank_assignments, matching but local_rank >= ranked_size). DO_NOT_SUBMIT: MODULE.bazel currently pins virtual-people-common via git_override to the open PR #75; replace with a normal version bump once #75 lands. Stacked on #89 (the bug fix that introduced the hash fallback). Related: #4073 * test: assert memoized_rank_fallback signal on no-match and empty paths Per review comments on #89: - NoMatchingRankAssignmentFallsBackToHash now asserts the flag fires. - NoMatchingRankAssignmentMatchesEmptyAssignmentsVid asserts both polarities: EXPECT_FALSE for empty-assignments (caller did not attempt memoization), EXPECT_TRUE for non-matching-assignments (caller attempted, leaf fell back). Matching changes in the Kotlin RankedLabelingIntegrationTest. * build: Bump virtual-people-common to 0.8.0 Replace the git_override pin on virtual-people-common#75 with the released 0.8.0 registry version now that it has landed, and update the lockfile accordingly. Also wrap an over-length comment in ranked_population_node_impl_test.cc to satisfy clang-format. --------- Co-authored-by: Steven Ware Jones <stevenwjones@meta.com> Co-authored-by: Steven Ware Jones <stevenwarejones@google.com>
|
This change is |
laureanobrs
force-pushed
the
laureanobrs-merge-virtual-people-core-serving
branch
from
July 21, 2026 19:20
da4173c to
42036f7
Compare
laureanobrs
force-pushed
the
laureanobrs-merge-virtual-people-core-serving
branch
from
July 21, 2026 20:00
54dc951 to
5fffa7c
Compare
This reverts commit 4d891c9.
laureanobrs
force-pushed
the
laureanobrs-merge-virtual-people-core-serving
branch
from
July 21, 2026 20:10
5fffa7c to
84a86a2
Compare
laureanobrs
force-pushed
the
laureanobrs-merge-virtual-people-core-serving
branch
from
July 21, 2026 21:20
bd53748 to
4d0f701
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.