Update NCF Dataset code

[angusrtaylor]

Description

• Fixed out-of-memory error: Users of the original code encountered out-of-memory errors when creating the Dataset object for large datasets. This is because the original code attempts to create a matrix of users and "negative items" (items the user has not interacted with) for negative sampling purposes. This matrix can have very large memory footprint. The updated code removes the need for creating this matrix by only identifying a user's negative item set when processing a user's data during training.
• Improved memory efficiency: The original code expands the original training dataset (containing only positive examples) by performing negative sampling for all users at the start of each training epoch and holding this full dataset in memory. This creates a memory bottleneck and causes problems when training on very large datasets. The updated code iterates over a csv file sorted by user and performs negative sampling for each user sequentially, never storing the full dataset in memory. A shuffle buffer is introduced to the train_loader function such that a subset of the full dataset is loaded into memory for shuffling purposes before returning batches. The size of the shuffle buffer is configurable.
• Fixed sampling problem: The original code samples user negative items every time it processes a positive example in the original training set. This is inefficient but also amounts to sampling with replacement which does not follow the approach of the original paper. The updated code samples negative items once per user without replacement. A new parameter is added to the Dataset class to enable sampling with replacement if desired.
• Fixed training/test set information leakage: in the original code, embeddings are created in the model for all users and items in both the training and test sets. This creates some information leakage for users/items that appear in the training set but not the test set. The updated code filters out unseen users/items from the test set in the notebook and additional filters are added to the Dataset object to prevent these users/items from being added to the test set.
• Fixed leave-one-out evaluation example: the original code runs the leave-one-out evaluation on the last 25% of the dataset using a chronological split which does not correspond to the approach taken in the original paper. The updated code aligns with the approach of the paper by only taking the last item for each user as the test set.

Note: updated unit tests are to follow.

Checklist:

• [ x] I have followed the contribution guidelines and code style for this project.
• [ x] I have added tests covering my contributions.
• [ x] I have updated the documentation accordingly.
• [ x] This PR is being made to staging branch and not to main branch.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[miguelgfierro]

looks great!

[miguelgfierro]

@angusrtaylor there are some errors with flake, are you able to access the logs?

[angusrtaylor]

@angusrtaylor there are some errors with flake, are you able to access the logs?

Thanks I'll address this

[anargyri]

Great job @angusrtaylor ! the code is really neat now.

[anargyri]

There was some error in the notebook, see here https://dev.azure.com/best-practices/recommenders/_build/results?buildId=56649&view=ms.vss-test-web.build-test-results-tab&runId=53844&resultId=100002&paneView=debug
It looks like it's in the construction of the Dataset.

[angusrtaylor]

There was some error in the notebook, see here https://dev.azure.com/best-practices/recommenders/_build/results?buildId=56649&view=ms.vss-test-web.build-test-results-tab&runId=53844&resultId=100002&paneView=debug It looks like it's in the construction of the Dataset.

Thanks. I saw that this morning but couldn't make sense of it. In my PR, the constructor is different (data = NCFDataset(train_file=train_file, test_file=leave_one_out_test_file, seed=SEED) in my branch vs data = NCFDataset(train=train, test=test, seed=SEED) in staging). So it seems the build is running the version of the notebook that's in staging rather than my branch.

[anargyri]

There was some error in the notebook, see here https://dev.azure.com/best-practices/recommenders/_build/results?buildId=56649&view=ms.vss-test-web.build-test-results-tab&runId=53844&resultId=100002&paneView=debug It looks like it's in the construction of the Dataset.

Thanks. I saw that this morning but couldn't make sense of it. In my PR, the constructor is different (data = NCFDataset(train_file=train_file, test_file=leave_one_out_test_file, seed=SEED) in my branch vs data = NCFDataset(train=train, test=test, seed=SEED) in staging). So it seems the build is running the version of the notebook that's in staging rather than my branch.

Strange. I am rerunning the job.

[miguelgfierro]

there is a weird OOM error:

E           /home/runner/work/recommenders/recommenders/.tox/gpu/lib/python3.7/site-packages/torch/utils/data/dataloader.py in _next_data(self)
E               561         data = self._dataset_fetcher.fetch(index)  # may raise StopIteration
E               562         if self._pin_memory:
E           --> 563             data = _utils.pin_memory.pin_memory(data)
E               564         return data
E               565 
E           
E           /home/runner/work/recommenders/recommenders/.tox/gpu/lib/python3.7/site-packages/torch/utils/data/_utils/pin_memory.py in pin_memory(data)
E                56         return type(data)(*(pin_memory(sample) for sample in data))
E                57     elif isinstance(data, collections.abc.Sequence):
E           ---> 58         return [pin_memory(sample) for sample in data]
E                59     elif hasattr(data, "pin_memory"):
E                60         return data.pin_memory()
E           
E           /home/runner/work/recommenders/recommenders/.tox/gpu/lib/python3.7/site-packages/torch/utils/data/_utils/pin_memory.py in <listcomp>(.0)
E                56         return type(data)(*(pin_memory(sample) for sample in data))
E                57     elif isinstance(data, collections.abc.Sequence):
E           ---> 58         return [pin_memory(sample) for sample in data]
E                59     elif hasattr(data, "pin_memory"):
E                60         return data.pin_memory()
E           
E           /home/runner/work/recommenders/recommenders/.tox/gpu/lib/python3.7/site-packages/torch/utils/data/_utils/pin_memory.py in pin_memory(data)
E                56         return type(data)(*(pin_memory(sample) for sample in data))
E                57     elif isinstance(data, collections.abc.Sequence):
E           ---> 58         return [pin_memory(sample) for sample in data]
E                59     elif hasattr(data, "pin_memory"):
E                60         return data.pin_memory()
E           
E           /home/runner/work/recommenders/recommenders/.tox/gpu/lib/python3.7/site-packages/torch/utils/data/_utils/pin_memory.py in <listcomp>(.0)
E                56         return type(data)(*(pin_memory(sample) for sample in data))
E                57     elif isinstance(data, collections.abc.Sequence):
E           ---> 58         return [pin_memory(sample) for sample in data]
E                59     elif hasattr(data, "pin_memory"):
E                60         return data.pin_memory()
E           
E           /home/runner/work/recommenders/recommenders/.tox/gpu/lib/python3.7/site-packages/torch/utils/data/_utils/pin_memory.py in pin_memory(data)
E                48 def pin_memory(data):
E                49     if isinstance(data, torch.Tensor):
E           ---> 50         return data.pin_memory()
E                51     elif isinstance(data, string_classes):
E                52         return data
E           
E           RuntimeError: cuda runtime error (2) : out of memory at ../aten/src/THC/THCCachingHostAllocator.cpp:280

[angusrtaylor]

cuda runtime error (2)

There are a bunch of tests that are failing in the build, even after merging in staging. I'm still trying to make sense of it. @anargyri's latest PR build was successful on staging so I'm trying to work out what is different compared to that...

[anargyri]

cuda runtime error (2)

There are a bunch of tests that are failing in the build, even after merging in staging. I'm still trying to make sense of it. @anargyri's latest PR build was successful on staging so I'm trying to work out what is different compared to that...

It could be something with the host. I rebooted and restarted the pipeline.

[angusrtaylor]

I figured out the problem with the NCF test: the failure was related to the ncf quickstart notebook which I had forgotten to update. I've updated it and pushed the changes. I think all the other failures are OOM errors.