Skip to content

[doc] fix grouped sequence_fields default rule; drop epoch/wall-clock note from train.md#542

Merged
tiankongdeguiji merged 2 commits into
alibaba:masterfrom
tiankongdeguiji:doc_seq_input_rule
Jun 11, 2026
Merged

[doc] fix grouped sequence_fields default rule; drop epoch/wall-clock note from train.md#542
tiankongdeguiji merged 2 commits into
alibaba:masterfrom
tiankongdeguiji:doc_seq_input_rule

Conversation

@tiankongdeguiji

@tiankongdeguiji tiankongdeguiji commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

What

Two doc fixes:

  1. docs/source/feature/feature.md — fix an inverted condition in the grouped sequence feature docs: for multi-input feature ops (LookupFeature, ComboFeature, etc.) without explicit sequence_fields, inputs with input_side == item default to sequence type — the doc previously said input_side != item.
  2. docs/source/usage/train.md — remove the confusing parenthetical「边界按 Unix 时间(epoch/墙钟)对齐,而非训练 epoch」from the save_checkpoints_timestamp_interval description; it used "epoch" in two senses (Unix epoch vs training epoch) in one sentence. The remaining text already describes the trigger; precise boundary semantics stay documented in checkpoint_util.py.

Why

For (1), the stated rule contradicted both the implementation and the doc's own example on the next line:

  • BaseFeature._is_sequence_input returns side == "item" for multi-input feature classes (tzrec/features/feature.py), mirroring the pyfg C++ rule in sequence_feature.cc.
  • The example already shows the correct behavior: item:cate maps to column click_seq__cate (sequence, prefixed) while user:kv_cate_cnt maps to kv_cate_cnt (non-sequence, unprefixed).

Verified live: a grouped lookup_feature with map: "user:kv_cate_cnt", key: "item:cate" and no sequence_fields yields sequence_input_names == ['click_seq__cate'].

🤖 Generated with Claude Code

tiankongdeguiji and others added 2 commits June 11, 2026 10:27
…e ops

For multi-input feature ops (LookupFeature, ComboFeature, etc.) inside a
grouped sequence feature, inputs with input_side == item default to
sequence type, not input_side != item. This matches
BaseFeature._is_sequence_input and the example on the next line
(item:cate -> click_seq__cate, user:kv_cate_cnt -> kv_cate_cnt).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…stamp_interval

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@tiankongdeguiji tiankongdeguiji changed the title [doc] fix grouped sequence_fields default rule for multi-input feature ops [doc] fix grouped sequence_fields default rule; drop epoch/wall-clock note from train.md Jun 11, 2026
@tiankongdeguiji tiankongdeguiji merged commit 93ad0e0 into alibaba:master Jun 11, 2026
6 of 7 checks passed
WhiteSwan1 added a commit to WhiteSwan1/TorchEasyRec that referenced this pull request Jun 11, 2026
…t/sid_abstract

upstream/master advanced to 3d4d5a8: alibaba#539 (SidRqkmeans) was merged as a squash
and alibaba#542 (docs) landed. feat already carries alibaba#539's content (from the earlier
pr-539 merge), so this is effectively the alibaba#542 doc update plus reconciling the
SID proto/types surface.

Conflicts (all add/add on files feat extended for RQ-VAE) resolved by taking
feat's superset:
- modules/sid/types.py: keep QuantizeForwardMode + ResidualQuantizerOutput
  (master's alibaba#539 has only QuantizeOutput, which is identical and auto-merged).
- protos/models/sid_model.proto: keep SinkhornConfig/ClipConfig/SidRqvae on top
  of the shared FaissKmeansConfig + SidRqkmeans.
- protos/model.proto: keep `SidRqvae sid_rqvae = 600;` (master still has it
  reserved as a comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants