[refactor] export sparse embeddings by table FQN#608
Conversation
…-table-fqn # Conflicts: # tzrec/utils/checkpoint_util.py # tzrec/utils/export_util_test.py
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| continue | ||
| table_fqn = name[: -len(".weight")] | ||
| export_emb_name = checkpoint_util.remap_input_tile_user_key(table_fqn) | ||
| state_values_by_emb[export_emb_name] = values |
There was a problem hiding this comment.
This is last-write-wins, so canonical selection depends on state_dict() order. EmbeddingGroupImpl registers ebc before ebc_user, meaning the remapped user tensor normally replaces the base tensor, while the new test uses the reverse order and expects the base value. Please explicitly prefer table_fqn == export_emb_name (falling back to an alias) and cover base-first order; otherwise a divergent or differently sharded user twin can silently become the exported physical table.
| for fname in t.feature_names: | ||
| feature_to_embedding_bag_info[fname] = t | ||
| table_fqn = ".".join( | ||
| filter(None, (child_path, "embedding_bags", t.name)) |
There was a problem hiding this comment.
Please add a focused test for this collector using real nested EBC/EC modules, including managed-collision recursion through ._embedding_module. The export-level test mocks this helper and lower-level tests hand-write FQNs, so nothing currently verifies that discovered table FQNs match actual state_dict() keys without .weight; a missing wrapper segment would leave the suite green but break export lookup.
|
Static review complete. I left two inline comments on deterministic INPUT_TILE alias selection and coverage of the new FQN collector. One general follow-up: please document the sparse artifact naming migration in The FQN direction and EC/EBC collision coverage otherwise look clear. Per request, I did not run tests or builds. |
tiankongdeguiji
changed the title
Reconciles master alibaba#604 (online dense export), alibaba#607 (track delta by owner-qualified FQN), alibaba#608 (export sparse by table FQN), and alibaba#609 (drop INPUT_TILE mapping) with the branch's FeatureStore delta-dump work. Keeps master alibaba#607's FQN-keyed delta tracking (local ModelDeltaTracker subclass, source/table_fqn schema) and alibaba#608's FQN sparse-export naming as the canonical base, and re-grafts the branch's FeatureStore upload, minute-cadence, and synced-dataloader-exhaustion onto alibaba#607's dumper. The uploader reads table_fqn and publishes remap_input_tile_user_key(table_fqn) as the FeatureStore embedding_name, aligning delta upload with alibaba#608's export naming and serving. Synced-exhaustion is kept so alibaba#607's _sync_final_step boundary-skip does not drop a shorter-exhausted rank's trailing delta (SEANQ). alibaba#604's online dense export and alibaba#609's INPUT_TILE removal are adopted as-is; main.py keeps the synced-exhaustion gating and FeatureStore start/close hooks. Co-Authored-By: Claude <noreply@anthropic.com>
2 participants
Summary
.weightremap_input_tile_user_key()__ec/__ebc) and EC pooling metadataNONE_user/ non-user conflict selectionmastercontrol flow1.3.7masterthrough03ef8856Why
Raw table names and synthetic role suffixes could not identify same-named tables owned by different modules, while INPUT_TILE twins could duplicate a single physical table in NPZ and sparse embedding metadata. Full table FQNs preserve genuine ownership boundaries, and existing INPUT_TILE key remapping makes aliases share one serving table.
Impact
Sparse NPZ keys, dynamic key/value/score names, sparse embedding JSON, and feature metadata use canonical physical table FQNs. User-side INPUT_TILE aliases no longer emit independent embedding entries. Same-named tables under unrelated owners remain distinct. INPUT_TILE model construction and checkpoint layout remain responsible for compatible, non-conflicting base/user aliases.
Test Plan
PYTHONPATH=. conda run -n tzrec130 python -m tzrec.utils.export_util_test(16 tests passed)remap_input_tile_user_keytests intzrec130(3 tests passed)1.3.7conda run -n tzrec130 pre-commit run -a(passed)python scripts/pyre_check.pyremains blocked because the checked-in.pyre_configurationis invalid JSON; a corrected scratch configuration reachespyre.bin, which requires glibc 2.33/2.34 while this host provides 2.32