Staging to main: xDeepFM to PyTorch, Movielens backup and more - #2370
Open
miguelgfierro wants to merge 40 commits into
Open
Staging to main: xDeepFM to PyTorch, Movielens backup and more#2370miguelgfierro wants to merge 40 commits into
miguelgfierro wants to merge 40 commits into
Conversation
Signed-off-by: Simon Zhao <simonyansenzhao@gmail.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
…ch/layers.py FcnNet, init_weight_ and the activation table are ports of base_model.py pieces that every deeprec model needs, not sequential-specific ones. Moving them out of sequential_base.py lets the upcoming xDeepFM port reuse them. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
The CIN normalizes a 3-D tensor over its last axis the same way the MLP head does, so the reshape belongs next to the other shared layers rather than inside FcnNet. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Same streaming text parsing as FFMTextIterator, but batches come out as numpy arrays laid out for embedding_bag instead of as a graph feed_dict. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Ports the linear, FM, CIN and DNN components as a standalone nn.Module. Verified against the TF model by copying its weights across: logits agree to float32 precision on all four components individually and combined, with and without batch normalization. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Covers the loader's parsing, field bagging and batching, the CIN's masked first layer, closed-form checks of the linear and FM logits, the additivity of the four components, and the fit/eval/predict/load lifecycle. Runs on synthetic data with no download. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
The PyTorch model replaces both. BaseIterator stays for DKN, the sequential models and newsrec, and no longer needs TensorFlow now that FFMTextIterator is gone. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Architecture goes on the constructor and training knobs on fit, so the yaml and prepare_hparams are no longer needed. Expected notebook metrics are refreshed from an actual run of the new model on Criteo. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Matches every other quick start, where the stored results are readable without running the notebook. Assigning the return of fit keeps the nn.Module repr out of the cell output. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
A typical config sets embed_l2 but leaves embed_l1 at zero, so every training step ran 0.0 * embedding.abs().sum() over the whole feature table. The guards sit inside the existing loops rather than splitting them, so the order of the float additions is unchanged and the results are bit-identical. Also drops the pred argument of _data_loss, which the default cross_entropy_loss branch never read, and with it a sigmoid computed on every step; the two branches that need a prediction now derive it themselves. The dnn_offsets prefix sum is written directly, and the per-layer DNN defaults follow the number of layers instead of assuming two. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
feature_count and FcnNet.enable_BN were assigned and never read; the FcnNet layers already collapse to Identity when batch normalization is off. The smoke test repeated its metric-name assertion verbatim and asserted an AUC is between 0 and 1. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
…tf-estimator-utils Remove tf_utils and the AzureML HyperDrive wide-and-deep notebook
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com> # Conflicts: # setup.py
Move protobuf out of core deps and increase limit on Transformers
…ables Follows the sasrec/model.py precedent: one self-contained model file per model, with its own sub-layers, instead of a neutral layers.py shared across model families. sequential_base.py and sli_rec.py go back to their staging state, so this PR no longer touches the SLi-Rec sources at all. The ACTIVATIONS dict and the init_method dispatch were leftovers of the TF yaml/hparams design this port removed. The DNN now calls F.relu directly (activation: [relu, relu] in every shipped config), the CIN applies no activation (cross_activation: identity everywhere), and weights use nn.init.trunc_normal_ directly (init_method: tnormal everywhere). The activation, cross_activation and init_method arguments are gone. Verified bit-identical logits against the previous commit for all four components with batch normalization enabled. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
The six regularization coefficients were dead state: _regular_loss() is only reached from the fit() training loop, which assigned all of them first, so the __init__ values could never be read. They are now arguments of _regular_loss(). batch_size and metrics were reachable, but only as an implicit fallback that made run_eval() and predict() behave differently depending on whether fit() had run. They become ordinary default arguments (batch_size=128, metrics=auc and logloss), and fit() passes its metrics down explicitly. Every call site in the repository already passes batch_size. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
xDeepFM and DKN reduce a flat feature vector to a logit with the same network: hidden layers of Linear -> [BatchNorm] -> [Dropout] -> activation, then a bare Linear(-, 1). DKN hand-rolls that network twice (its scorer and its attention head) and xDeepFM carried its own copy, so it becomes one class. The activation is an nn.Module argument and the initializer a callable, because the two models genuinely differ there: xDeepFM scores with ReLU and a truncated normal init, while DKN ships sigmoid on the scorer, ReLU on the attention head and a uniform init. Neither is a string switch. dropout carries one rate per hidden layer and 0.0 disables it, so there is no separate on/off flag; a length mismatch with layer_sizes is rejected up front. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
…otes Uses the shared FcnNet and removes the redundant knobs a config-driven design had left behind: - user_dropout: nn.Dropout(p=0.0) is already an identity and [0.0, ...] is already the default, so the flag only made dropout=[0.5, 0.5] silently do nothing. - is_clip_norm and save_model: max_grad_norm=None and model_dir=None express "off" on their own. - The dim argument of CIN: read once, to shape a reshape that flatten(2) does without it. - self.device: a second source of truth that model.to() desynchronised, which is why the tests had to write it by hand. It is now a read-only property over the device of the embedding table. - The four dtype conversions in _to_tensors: the loader already emits those dtypes, and dnn_offsets is now explicitly int64 so that stays true. - The data_size yielded by the loader, equal to len(labels); its col_spliter and ID_spliter, unreachable because the model constructs FFMDataset itself; the unread self.seed and the cuda seeding call that torch.manual_seed already covers; and the field_nums list in CIN, whose first entry never changed. fit now rejects an unknown loss before the first batch instead of mid-epoch, and its per-epoch line no longer labels every loss "logloss". Docstrings and comments describe what the code does rather than the TensorFlow implementation it came from; the paper citation stays. Verified bit-identical prediction scores against the previous commit, end to end through the loader, with all four components and batch normalization enabled. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
test_xdeepfm.py covered three modules at once. The loader tests move to test_ffm_dataset.py, and fcn_net.py gets test_fcn_net.py, which it never had: the head was only exercised through the model, so its activation, its initializer callable, its dropout in train and eval mode, and the batch-norm switch had no direct coverage. All three files are registered in test_groups.yml. The quick-start notebook is re-executed. Every metric is unchanged, digit for digit, including all ten per-epoch losses and the final test auc 0.7355 / logloss 0.5014; the only difference in the output is the per-epoch line, which no longer labels a cross_entropy_loss value "logloss". Docstrings drop the "(PyTorch)" qualifier now that there is no other version to distinguish it from. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
…a conftest The construction helpers become factory fixtures: build_model and build_fcn_net return a callable so each test can override what it needs, which a plain fixture cannot do. first_batch and field_embeddings follow, so no private module helper is left in either file. init_weight=nn.init.ones_ replaces the wrapper that only forwarded to it. Splitting the loader tests out had duplicated the synthetic FFM writer across two files. It moves to tests/unit/recommenders/models/conftest.py as the synthetic_ffm fixture, next to the existing evaluation/conftest.py. Both files now read the same train/valid/test files instead of writing their own. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
… original URL Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
The notebook downloaded u.data straight from the GroupLens URL, bypassing download_movielens and its Hugging Face backup. It now goes through load_pandas_df, so the download falls back to the backup when GroupLens is unreachable. The Spark section builds its DataFrame from the same pandas DataFrame instead of re-reading the raw file. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
MovieLens tests belong in tests/data_validation/recommenders/datasets/test_movielens.py, where they were consolidated. This file re-created a path that had been vacated. Signed-off-by: miguelgfierro <miguelgfierro@users.noreply.github.com>
…ns-hf-backup Add Hugging Face backup URL for MovieLens downloads
…-pytorch Migrate xDeepFM from TensorFlow to PyTorch
miguelgfierro
requested review from
SimonYansenZhao,
anargyri,
loomlike and
wav8k
as code owners
September 4, 2026 15:06
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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
2 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.
Description
Related Issues
References
Checklist:
git commit -s -m "your commit message".staging branchAND NOT TOmain branch.