[feat] SID: add offline codebook collision-prevention tool#602
Conversation
Deterministic offline post-process over predicted SID rows that caps items per SID bucket at --max_items_per_codebook and reassigns the overflow. Strategies: - candidate (default): reassign to explicit nearest-neighbor candidate rows. - random: draw a uniform-random free within-band last-layer code, no candidate input required; a baseline that ignores semantic nearest-neighbor proximity and is still reproducible given --seed. Local (CSV/Parquet) and ODPS-SQL backends. Capacity/strategy are CLI-only policy (deliberately kept out of all protos). Output columns: item_id, origin_codebook, codebook, index. Includes unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…p ODPS SQL Follow hitrate.py -- route every backend through create_reader/create_writer (which already speak CSV/Parquet/ODPS) instead of a parallel hand-written ODPS-SQL path. Removes OdpsTableRef, OdpsSqlGenerator (~264 lines of SQL generation), OdpsCollisionRunner, and the generate_odps_sql/run_odps entry points. LocalCollisionRunner -> CollisionRunner (now serves ODPS too via --reader_type/--writer_type OdpsReader/OdpsWriter); run_local -> run; drop the --backend / --temp_prefix / --odps_lifecycle / --dry_run_sql CLI surface. SidCollisionAssigner (the assignment algorithm) is unchanged. Tool 1231 -> 814 lines; tests 588 -> 441 (dropped the 4 ODPS-SQL tests). 14 tests pass; ruff and format clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- extract CollisionRunner._write_table (create_writer/write/close) so the assignments and diagnostics writers share one path; plain dict instead of OrderedDict (dicts are ordered on the 3.10 target). - drop the unused --candidate_origin_codebook_field CLI arg -- an orphan from the removed ODPS-SQL path, never read by the runner. 14 tests pass; ruff + format clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_candidate_sort_key's blake2b tie-breaker is pure but was recomputed on every pass of the up-to-max_iters assignment loop -- in the per-codebook sorts and the two-sided best-candidate comparisons. Cache it on the frozen CandidateSidRow so each unique key is hashed exactly once, cutting the assignment phase from O(max_iters x N) blake2b digests to O(N). Output is identical (same keys, same order). 14 tests pass (incl. the determinism tests); ruff + format clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pass selected_cols=[item_id, *code_fields] into create_reader on the raw SID read, so wide / ODPS source tables no longer decode unused columns (create_reader already forwards selected_cols to the reader; hitrate.py does the same). The candidate read still reads all columns because its priority/score fields are optional and projecting them would fail when absent. 14 tests pass; ruff + format clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror hitrate.py -- capture the main-input reader class during the raw read and default --writer_type to it (CsvReader -> CsvWriter, etc.) instead of hard-defaulting to ParquetWriter, so the output backend matches the input's when unspecified. Explicit --writer_type still wins, and ODPS output still resolves to OdpsWriter via create_writer's path detection. Adds a test for the derivation path. 15 tests pass; ruff + format clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…table The SID model emits `codes` and the candidate SIDs in the same rows, so drop --candidate_input_path (and the now-redundant --candidate_reader_type / --candidate_item_id_field): raw SIDs and candidates both come from --input_path, read in one pass. Candidates are loaded only for --strategy candidate (random synthesizes its own) and only when the candidate column is present, so a plain SID table still runs (overflow then follows --unassigned_policy). Merges the two loaders into _load_rows with _origin_codes / _candidate_rows helpers; tests use single-table fixtures. 15 tests pass; ruff + format clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… only) The SID lands in one `codes` column (a scalar string or a list cell), so remove the split-across-columns --code_fields path along with its _code_fields helper and the now-trivial _origin_codes helper (folded into the read loop). --code_field (default "codes") is the sole origin-SID source. 15 tests pass; ruff + format clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pass stats) Byte-identical, deterministic-output-preserving cleanups from the perf review: - slots=True on the four row dataclasses (~2x per-object memory); item_key becomes a derived @Property on RawSidRow/AssignedSidRow (was a duplicated str(item_id) field). - _dedup_candidates filters to overflow_items (only their candidates can ever be used), capping the dedup map + sort-key memo to the overflow fraction. - intern codebook strings (heavy repetition over a small distinct-SID space). - fold the stats passes (raw_collision_buckets from by_origin; reassigned + final counts in one loop); drop the throwaway dup-detection set (build the dict first); in-place assigned.sort(); heapq.nsmallest for the top-k trims. - _write_diagnostics via dataclasses.asdict; assign_sid_collisions collapsed to **kwargs (defaults live only on the class). Verified byte-identical by a 6-scenario CLI harness (compact / 1:1+score / score-order / drop / keep_original / random, with real multi-iteration reassignment) that hashes identically before and after; 15 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rework SidCollisionAssigner's reassignment loop so candidates are grouped by codebook and sorted ONCE (the sort key is a total order) instead of re-grouping and re-sorting every codebook on each of up to max_iters passes. Each pass now scans the pre-sorted lists filtered by a live `unassigned` set (discard on assignment), rather than rebuilding `set(raw_by_item) - assigned_items` over all items every pass; _available_candidates is deleted and _handle_unassigned reuses the same set. The assignment phase drops from ~O(passes x N x K) to ~O(one sort + scans). Output is byte-identical: best-per-item and the final accepted sort are order-independent, and unique keys make filter-then-sort == sort-then-filter. Verified by a 6-scenario CLI harness (identical output hash) and 15 unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…w items) Split _load_rows into two passes over the single input table: pass 1 reads just [item_id, code] (projected) to build raw rows + per-origin counts; pass 2 reads the candidate columns and materializes a CandidateSidRow only for items in over-capacity buckets -- the only ones that can overflow. Candidate memory now scales with the overflow fraction instead of one row per candidate per item (the difference between OOM and feasible at 10M x 64). Also inlines the single-call _read_batches into _read (single-table -> paths come from self.args) and folds candidate-field resolution into _candidate_field. Output byte-identical: over-capacity buckets are a deterministic superset of the true overflow, which the assigner's dedup already narrows, so the assigner sees the same candidates. Verified by the 6-scenario CLI harness (identical hash) and 15 unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…SV compat) Represent SID codebooks as tuple[int] (the codes are bigints) rather than a delimited string. The tool uses a codebook purely as an opaque hashable, ordered bucket identity, so a tuple is the faithful form. I/O: - Parquet/ODPS (primary): read/write list<int64> columns directly -- codes, candidate_codebook, and the compact-candidate field (list<list<int64>>); origin_codebook/codebook are written as list<int64>. - CSV (fallback): a compatibility layer parses delimited-int strings on read and joins codes with --code_delimiter on write (chosen by the writer type). Drops the string join/parse/intern path, makes the tie-break numeric (natural SID order), and hands downstream the integer codes with no re-parse; random candidate generation is now pure tuple ops. Determinism preserved. Verified that CSV and Parquet with the same logical data normalize to identical tuples and produce identical assignment decisions (a unit test plus a 3000-item cross-backend check); 16 tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the per-row object-based iterative allocator with a numpy/Arrow vectorized single greedy pass that reassigns only the last SID layer within an item's band. The object path materialized a dataclass plus candidate objects per row and drove an up-to-50-pass allocation, which does not scale to the hundred-million-row maps this tool targets; the vectorized path packs band keys to int64, ranks with lexsort/unique, and runs one first-fit pass, keeping the multi-backend reader/writer and adding chunked writes plus --rate_only. The greedy single pass replaces ALGR's iterative multi-pass allocation, trading a small placement-quality loss in near-saturated bands for a ~15x faster, vectorizable, contention-insensitive pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…config Declare the SID shape with a required --codebook (comma-separated per-layer sizes) instead of inferring n_layers, band-packing radices, and the last-layer code space from the data; this removes the "batch missing the max code" edge case and subsumes --random_last_layer_size (now codebook[-1]). --seed becomes a fixed module constant since the collision rate is seed-invariant. Config args are parsed into private attributes and validated at construction; the vectorized paths pick up review cleanups (in-place band packing, Python-native candidate keys, a param-less _read). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rename the CollisionPlan/CollisionResolutionResult arrays for clarity (origin_bucket_indices, bucket_keys/bucket_counts, final_bucket_keys, overflow_bucket_key_prefixes), document CollisionPlan's fields, and remove test-only properties, single-use helpers, and redundant guards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the diagnostics-output feature (--diagnostics_output_path, the _write_diagnostics writer, and the CollisionResolutionStats.to_output_dict serializer), and with it the now-redundant reassigned_count/unassigned_count stat aliases whose only purpose was the ALGR-named diagnostics columns; the stats object now speaks one vocabulary (relocated_count/unresolved_count). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…prevention Resolve the tzrec/tools/sid/__init__.py add/add conflict by keeping the package docstring; upstream's sid_quality_report joins our collision tools in the same dir. No proto changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These were a design proposal for an unimplemented append mode and referenced the local workspace; not part of the collision-prevention tool contribution. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the error and drop policies (and the --unassigned_policy config): an overflow item that cannot be relocated now always keeps its original SID over capacity, so every input item is preserved. Removes the retained_mask machinery that only existed for the drop policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the keep-original assignment into the placement loop's else branch (the separate post-loop pass was residue of the removed multi-policy dispatch), fix stale "retained" wording in the resolved-groups CLI help, and bump the version to 1.3.5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The SID models emitted candidate_codes as nested list<list<int>> (topk x n_layers), which pyarrow's CSV writer cannot serialize and which forced the offline collision tool to carry two bespoke decoders plus dedicated "|"/";" separators. Flatten it to a single list<int> of topk*n_layers codes in _sid_predictions, so candidates share the same comma-separated wire format and the same _codes_matrix decoder as the primary codes column; the tool recovers per-candidate structure by splitting on the passthrough --codebook length. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tiankongdeguiji
left a comment
There was a problem hiding this comment.
Please restructure the SID tooling as follows:
tzrec/tools/sid/
├── resolve_sid_collisions.py
└── evaluate_sid_quality.py
tzrec/utils/sid/
├── collision.py
├── quality.py
└── catalog.py # Future; do not add in this PR
Specifically:
- Rename
collision_prevention.pytoresolve_sid_collisions.py. - Move reusable logic from
collision_resolution.pyintoutils/sid/collision.py. - Rename
sid_quality_report.pytoevaluate_sid_quality.pyand move reusable metrics intoutils/sid/quality.py.
Please preserve the current two-pass processing design:
plan = prepare_collision_plan(item_ids, origin_codes, config)
# Only required by KnnCollisionResolver.
candidate_codes = load_candidate_codes(
candidate_input_path,
plan.overflow_item_ids,
)
result = resolver.resolve(plan, candidate_codes)The first pass should build a compact CollisionPlan. For KNN resolution, the second pass should load candidates only for plan.overflow_item_ids. The random strategy should skip candidate loading.
utils/sid/collision.py should provide:
CollisionPlan
prepare_collision_plan()
CollisionResolver
KnnCollisionResolver
RandomCollisionResolver
The resolver interface should operate on the prepared plan:
class CollisionResolver(ABC):
@abstractmethod
def resolve(
self,
plan: CollisionPlan,
candidate_codes: np.ndarray | None = None,
) - CollisionResolutionResult:
"""Resolve collisions for a prepared overflow plan."""evaluate_sid_quality.py should support both raw assignments and the output of resolve_sid_collisions.py. When both origin_codes and resolved codes are present, it should report before/after metrics and their deltas.
Please also document that the current resolvers are best-effort: a bucket may remain over capacity when all candidates are exhausted.
| --codebook 256,256,256 --max_items_per_codebook 5 \ | ||
| --strategy candidate \ | ||
| --output_path sid_collision/map \ | ||
| --original_sid_groups_output_path sid_collision/original_groups \ |
There was a problem hiding this comment.
Make original_sid_groups_output_path optional. The original grouping is primarily a diagnostic/audit artifact, while the item map and resolved grouping are the operational outputs consumed downstream.
…helpers Co-locate the algorithm unit tests under tzrec/utils/sid/ and tighten the quality/collision code: the quality CLI routes its mirrored before/after decode-validate-tally through one _decode_and_validate helper and derives malformed_rows instead of storing it; the collision greedy loop pre-materializes its overflow columns to avoid a per-row int() boxing of three numpy arrays. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_multi_tower_with_fg_train_eval_export_trt compares TRT vs non-TRT predictions within a 2e-5 tolerance; TRT tactic selection is nondeterministic and unrelated to this branch's SID-only changes. Empty commit re-runs the lane. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The global uniqueness check and duplicate overflow rejection blocked large jobs on rare upstream duplicate IDs. Reuse the matched candidate list across duplicate overflow rows while preserving the input rows and collision accounting.
|
Static review only; no tests or builds were run. I left seven inline comments on the remaining noteworthy issues:
The PR description should also be updated: it currently calls the output capacity-safe, while the implementation intentionally keeps unplaceable items in over-capacity origin buckets. The module and CLI already describe that best-effort behavior accurately. |
Share path conflict validation across SID tools and bound collision-processing temporaries without changing greedy placement order.
|
thanks for github-actions's comment. Changes are included in Accepted
Rebutted
Follow-up
SummaryAll correctness and low-risk memory findings were addressed. Contract-only cases were documented, while architectural performance work remains scoped to future PRs. |
Summary
This PR adds TorchEasyRec-native tools for evaluating Semantic ID (SID) quality and resolving over-capacity SID buckets offline:
tzrec.tools.sid.evaluate_sid_qualitycomputes exact global and per-layer quality metrics. It can compare original and resolved fields on the same rows and emit before/after/delta reports.tzrec.tools.sid.resolve_sid_collisionsperforms deterministic, best-effort collision resolution within each SID prefix band. It supports model-provided candidates and the legacy deterministic random strategy.topk * n_layerswire format consumed by the resolver.tzrec/utils/sid/quality.pyandtzrec/utils/sid/collision.py; tool modules contain repository I/O and CLI orchestration.tzrec/utils/path_util.pyand reports both conflicting groups and their normalized overlapping location.Both tools use the repository
create_reader/create_writerinterfaces and support CSV, Parquet, and ODPS. Collision resolution is single-process and CPU-only.Input and output contract
The resolver consumes fixed-width
tzrec.predictoutput:item_id, which is expected to be unique upstream;codes, alist<int>of widthn_layers;--strategy candidate,candidate_codes, a flatlist<int>of widthtopk * n_layers.It writes:
item_id,origin_codebook,codebook, and the one-basedindexwithin the final SID bucket;codebookanditemids;--rate_onlycomputes statistics without writing outputs. With unique item IDs, results are independent of source-row order and I/O batch boundaries. Rare duplicate IDs are preserved as independent rows on a best-effort basis; they should still be corrected upstream.Resolution is intentionally best-effort. An overflow item takes its first free last-layer candidate while retaining its prefix. If its finite candidate list has no free destination, it keeps its original SID. The bucket and its
indexmay therefore remain above--max_items_per_codebook; this is reported throughunresolved_countand final collision statistics.Design decisions
tzrec.predictproducer contract. Arbitrary malformed external Arrow tables are not a supported input format in this PR.Test plan
The five focused modules run 145 tests covering golden resolution and quality results, CSV/Parquet/mocked ODPS I/O, before/after comparison, duplicate-ID tolerance, no-overflow and unresolved fallback paths, local/ODPS path aliases, cross-batch candidate alignment, Arrow offset-aware writes, progress reporting, and equivalence across first-fit chunk boundaries. All focused tests and
pre-commit run -apass. The local Pyre wrapper reports no critical type errors; the installed Pyre CLI also emits a version-option compatibility warning.