Skip to content

Migrate SLi-Rec from TensorFlow to PyTorch - #2358

Open
miguelgfierro wants to merge 17 commits into
stagingfrom
miguelgfierro/slirec-pytorch
Open

Migrate SLi-Rec from TensorFlow to PyTorch#2358
miguelgfierro wants to merge 17 commits into
stagingfrom
miguelgfierro/slirec-pytorch

Conversation

@miguelgfierro

@miguelgfierro miguelgfierro commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Description

Migrates the SLi-Rec sequential recommender from TensorFlow (TF-1.x graph mode) to PyTorch, as part of the TF→PyTorch migration tracked in #2302.

The port is a self-contained nn.Module mirroring the existing PyTorch graphrec/lightgcn.py and the lightgcn_deep_dive notebook: all hyper-parameters are explicit arguments — no prepare_hparams/yaml/HParams object. Architecture goes on the constructor, training knobs on fit, evaluation knobs on run_eval; it reuses only the framework-agnostic metric functions in deeprec_utils. The TensorFlow SLi-Rec is removed and replaced by this port; the five other TF sequential models (asvd, caser, gru, nextitnet, sum) and the shared TF base are untouched.

model = SLI_RECModel(
    user_vocab=user_vocab, item_vocab=item_vocab, cate_vocab=cate_vocab,
    item_embedding_dim=32, cate_embedding_dim=8, user_embedding_dim=16,
    hidden_size=40, attention_size=40, max_seq_length=50,
    layer_sizes=[100, 64], att_fcn_layer_sizes=[80, 40], dropout=[0.3, 0.3], seed=SEED,
)
model.fit(train_file, valid_file, epochs=10, batch_size=400, learning_rate=0.001,
          train_num_ngs=4, valid_num_ngs=4, embed_l2=0.0, layer_l2=0.0)
model.run_eval(test_file, num_ngs=9)   # -> {"auc": ..., "group_auc": ..., ...}

New files

  • recommenders/models/deeprec/models/sequential/pytorch/rnn_cell_pytorch.pyTime4LSTMCell + a dynamic_rnn-equivalent scan driver.
  • recommenders/models/deeprec/models/sequential/pytorch/sequential_base_pytorch.py — shared embeddings, unmasked ASVD Attention, FcnNet MLP head, softmax pairwise loss, unique-embedding regularization, and the fit/run_eval/predict lifecycle.
  • recommenders/models/deeprec/models/sequential/pytorch/sli_rec_pytorch.pySLI_RECModel (forward == the TF _build_seq_graph) + masked AttentionFcn.
  • recommenders/models/deeprec/io/sequential_dataset_pytorch.py — faithful port of SequentialIterator (parsing, time features, padding, in-batch negatives).
  • tests/unit/recommenders/models/test_sli_rec_pytorch.py — 12 plain-function unit tests.
  • examples/00_quick_start/slirec_amazon.ipynb — a dedicated PyTorch SLi-Rec quickstart that defines parameters as plain Python and passes them explicitly (no prepare_hparams), so the functional test still exercises SLi-Rec end-to-end.

Removed (TensorFlow SLi-Rec)

  • recommenders/models/deeprec/models/sequential/sli_rec.py and its TF-only time-aware cell file rnn_cell_implement.py (used exclusively by TF SLi-Rec).
  • The TF SLi-Rec unit (test_slirec_component_definition) and smoke (test_model_slirec) tests and their test_groups.yml entries.
  • docs/models.rst: the RNN Cells section; the SLIRec section now points at the PyTorch modules.

Changed

  • tests/conftest.py — the slirec_quickstart notebook fixture points at the new slirec_amazon.ipynb; README links updated.
  • examples/00_quick_start/sequential_recsys_amazondataset.ipynb — kept as the shared multi-model quickstart, but its default active model is switched from the removed TF SLi-Rec to A2SVD (same IJCAI'19 paper), with a pointer to the new SLi-Rec notebook. The other TF sequential models are unaffected.

Validation

Component-level parity (weights copied TF→PyTorch, on the real slirec data):

  • Time4LSTMCell + scan: single-step m/c diff ~1e-7; full padded-sequence rnn_outputs diff ~6e-8; padded steps exactly zero.
  • SequentialDataset: parser, eval-batch arrays, and train-batch arrays (incl. in-batch negative sampling under a shared RNG seed) all bit-identical to the TF iterator.

End-to-end training (10 epochs, batch 400, seed 42, embed_l2=layer_l2=0 — the functional-test config):

untrained epoch 1 (valid) epoch 10 (valid) test
PyTorch AUC 0.4802 0.5071 0.7614 0.7355
TF AUC 0.4857 0.4975 0.7369 0.7174

The functional test asserts auc == pytest.approx(0.7183, rel=0.1, abs=0.05) → accepts [0.6465, 0.7901]. The PyTorch test AUC 0.7355 passes comfortably and exceeds both the 0.7183 target and the TF reference (group_auc 0.7201 vs TF 0.7073). Unit tests: 12/12 pass on CPU. The untrained baseline AUC (0.4802) is bit-identical to the earlier hparams-based version, confirming the explicit-params refactor preserves initialization and behavior.

Related Issues

Checklist

  • I have not rewritten tests or code to pass CI without addressing the underlying issue.
  • I ran black to format the code.
  • I have added tests (unit tests + the existing functional notebook test now exercises the PyTorch model).
  • The five other TF sequential models are untouched (byte-identical to staging); their tests still collect.

Port the time-aware LSTM cell (Time4LSTMCell) and a dynamic_rnn-equivalent
scan driver from TF to PyTorch as the first component of the SLi-Rec migration.
Weight-copied parity vs the TF cell: single-step m/c diff ~1e-7, full padded
sequence rnn_outputs diff ~6e-8. Includes the migration design doc.

Signed-off-by: miguelgfierro <[email protected]>
Faithful port of SequentialIterator (parser_one_line, load_data_from_file,
_convert_data) without TF placeholders. Verified bit-identical to the TF
iterator on real slirec data: parser, eval-batch arrays, and train-batch
arrays including in-batch negative sampling under a shared RNG seed.

Signed-off-by: miguelgfierro <[email protected]>
Standalone nn.Module SLi-Rec (mirroring graphrec/lightgcn.py's API) built on a
reusable SequentialBaseModel: shared user/item/cate embeddings, unmasked ASVD
Attention, masked AttentionFcn, FcnNet MLP head (Linear->BN->Dropout->Activation),
softmax pairwise loss, unique-embedding regularization, and fit/run_eval/predict/
load_model reusing deeprec_utils.cal_metric. Verified end-to-end on real slirec
data: untrained baseline auc~0.48 (TF ~0.4857), loss starts at the random value
~1.61 and decreases, epoch-1 valid auc 0.513. Applies black to the new modules.

Signed-off-by: miguelgfierro <[email protected]>
Plain-function pytest (no classes) covering: Time4LSTMCell gate-equation parity
and time-column asymmetry, scan padding zeroing, ASVD/masked attention behavior,
FcnNet 2D/3D shapes, dataset time-feature/mask/negative-sampling structure, the
E==H dimension-coupling guard, model forward shape, and a fit+eval smoke run on
tiny synthetic vocabs/data (no Amazon download; CPU-friendly). 12 passed.

Signed-off-by: miguelgfierro <[email protected]>
Repoint cell 3 imports to the PyTorch SLI_RECModel and SequentialDataset (aliased
as SequentialIterator so input_creator is unchanged); drop the tensorflow import
and print torch's version instead. Every other cell (prepare_hparams, fit,
run_eval, predict, load_model, store_metadata) is unchanged, so the functional
test test_slirec_quickstart_functional runs as-is. Public API preserved.

Signed-off-by: miguelgfierro <[email protected]>
Component parity (~1e-7) and end-to-end training: PyTorch test AUC 0.7361 vs
target 0.7183 and TF ref 0.7174, passing the functional-test tolerance.

Signed-off-by: miguelgfierro <[email protected]>
@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Drop docs/superpowers/specs/2026-07-13-slirec-pytorch-migration-design.md from
the tree and add docs/superpowers/ to .gitignore so generated design docs are
never committed.

Signed-off-by: miguelgfierro <[email protected]>
SLi-Rec is now provided by the PyTorch port, so remove the TF version and its
TF-only time-aware cell file (used exclusively by TF SLi-Rec):
- delete sequential/sli_rec.py and sequential/rnn_cell_implement.py
- drop the TF slirec unit (test_slirec_component_definition) and smoke
  (test_model_slirec) tests and their test_groups.yml entries
- docs/models.rst: remove the RNN Cells section and repoint SLIRec at the
  PyTorch modules

The other five TF sequential models (asvd, caser, gru, nextitnet, sum) are
unaffected; their tests still collect.

Signed-off-by: miguelgfierro <[email protected]>
… models

Rather than repurposing the shared sequential quickstart, add a dedicated
examples/00_quick_start/slirec_amazon.ipynb for the PyTorch SLi-Rec (cleared
outputs, SLi-Rec-only imports). Restore sequential_recsys_amazondataset.ipynb
to its original form and switch its default active model from the now-removed
TF SLi-Rec to A2SVD (same IJCAI'19 paper), with a pointer to the new notebook.
Repoint the slirec_quickstart fixture and the README links at slirec_amazon.ipynb.

Signed-off-by: miguelgfierro <[email protected]>
Drop the prepare_hparams/HParams/yaml dependency from the PyTorch SLi-Rec API in
favor of explicit arguments, mirroring examples/.../lightgcn_deep_dive.ipynb:

- SLI_RECModel(user_vocab, item_vocab, cate_vocab, item_embedding_dim=32,
  cate_embedding_dim=8, hidden_size=40, attention_size=40, layer_sizes=[100,64],
  att_fcn_layer_sizes=[80,40], dropout=[0.3,0.3], ...): architecture on the ctor.
- fit(train_file, valid_file, epochs, batch_size, learning_rate, train_num_ngs,
  valid_num_ngs, embed_l2, layer_l2, ...): training knobs on fit.
- run_eval/predict take batch_size + metric lists explicitly.
- SequentialDataset takes vocab paths + max_seq_length; batch_size moves to
  load_data_from_file. The model builds the loader internally.
- Notebook slirec_amazon.ipynb defines params as plain Python and passes them
  explicitly; the functional test drops the yaml_file param.

Behavior preserved: untrained baseline AUC 0.4802 is bit-identical to the
hparams version (same init/RNG order); unit tests 12/12 pass.

Signed-off-by: miguelgfierro <[email protected]>
Comment thread examples/00_quick_start/sequential_recsys_amazondataset.ipynb Outdated
Comment thread examples/00_quick_start/slirec_amazon.ipynb
A2SVDModel._build_seq_graph reads hparams.attention_size, but asvd.yaml
never defined it, so running A2SVD with its own config file raised
AttributeError. Use 40, the same value as sli_rec.yaml.

Signed-off-by: miguelgfierro <[email protected]>
The pytorch subpackage already disambiguates the implementation, so
sli_rec_pytorch.py, rnn_cell_pytorch.py, sequential_base_pytorch.py,
sequential_dataset_pytorch.py and test_sli_rec_pytorch.py lose the
suffix. All imports, docs and the quickstart notebook are updated.

Signed-off-by: miguelgfierro <[email protected]>
@miguelgfierro

Copy link
Copy Markdown
Collaborator Author

@anargyri I updated the code, please take a look

assert logit.shape == (n, 1)


def test_slirec_fit_and_eval_smoke(synthetic_slirec):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these methods used anywhere?

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