Repository: NVIDIA-Merlin/dataloader (language: python)

Issue:
Update TensorFlow Loader to run as iterator instead of Keras Sequence
Updating the TensorFlow Loader to act as an iterator instead of a Keras Sequence. And the loader to act as an iterator for torch as well.

In TensorFlow 2.10 the way the keras seqeuence data handler changed. The `__len__` method is no longer the first method called at the start of an epoch which breaks our implementation of the Sequence, because it has a side effect of resetting the loader. Resulting in a `StopIteration` exception being raised when using the loader.

This can be worked around by implementing the `on_epoch_end` method to call stop on the loader. However, the loader implementation would remain prone to error in future if keras changed it's data handler implementation to not call `__len__` prior to starting an epoch and continued to check one of the batches with `__getitem__`. Since the Loader also doesn't behave like a keras sequence. A keras sequence is expected to be able to lookup a batch by index with `__getitem__` without changing any iterator state. The current implementation calls `next` to advance the iterator.

This change removes the keras Sequence from the Loader class and the methods `__len__` and `__getitem__`. When using keras after this change, a different data adapter will be used - [GeneratorDataAdapter](https://github.com/keras-team/keras/blob/v2.10.0/keras/engine/data_adapter.py#L828) instead of the [KerasSequenceAdapter](https://github.com/keras-team/keras/blob/v2.10.0/keras/engine/data_adapter.py#L828).

# Epoch Example

When `.fit()` or `.evaluate()` is called on a model and using a Keras Sequence as an input. Prior to the start of the epoch there is some preliminary work that goes on to inspect a batch and check the length of the input data. Followed by the main loop through all the batches.

The following demonstrates the ordering of method calls during a call to `.fit()`.

## TensorFlow/Keras 2.9

```
__len__
__getitem__
Epoch 1/2
__len__
__getitem__
__getitem__
__getitem__
on_epoch_end
```

## TenorFLow/Keras 2.10/2.11

```
__getitem__
__len__
__len__
Epoch 1/2
__len__
__getitem__
__getitem__
__getitem__
on_epoch_end
```

## Example - Motivation for moving from sequence to iterator

Implementing `on_epoch_end` to call `.stop()` makes our sequence-based loader work with tensorflow 2.10/2.11.

However, this implementation remains open to future bugs if for example keras were updated to call the methods in the following order when running fit or evaluate.

If the call to `__len__` were removed at the start of the epoch (since len has already been called and we have that information, it could in theory be removed), then the iterator would have advanced by one prior to the start of the epoch and we'd either silently drop the first batch or get an exception triggered by `StopIteration`.

```
__len__
__getitem__
Epoch 1/2
__getitem__
__getitem__
__getitem__
on_epoch_end
```
[WIP] add examples
This is a WIP branch for adding examples. I have added 01 (and a brief README) and am looking for feedback if I am moving in the right direction here.

Thank you for all your help! 🙂


Discussion / hints:

Check out this pull request on&nbsp; <a href="https://app.reviewnb.com/NVIDIA-Merlin/dataloader/pull/47"><img align="absmiddle"  alt="ReviewNB" height="28" class="BotMessageButtonImage" src="https://raw.githubusercontent.com/ReviewNB/support/master/images/button_reviewnb.png"/></a> 

 See visual diffs & provide feedback on Jupyter Notebooks. 

---

 <i>Powered by <a href='https://www.reviewnb.com/?utm_source=gh'>ReviewNB</a></i>
## Documentation preview

<https://nvidia-merlin.github.io/dataloader/review/pr-47>

It looks good - I added some comments.

We need a unittest for the final notebooks
rerun tests

Fix patch:
diff --git a/examples/01a-Getting-started-Tensorflow.ipynb b/examples/01a-Getting-started-Tensorflow.ipynb
index f8807aa8..fd558064 100644
--- a/examples/01a-Getting-started-Tensorflow.ipynb
+++ b/examples/01a-Getting-started-Tensorflow.ipynb
@@ -326,8 +326,7 @@
     }
    ],
    "source": [
-    "batch = loader.peek()\n",
-    "batch"
+    "loader.peek()"
    ]
   },
   {
@@ -354,7 +353,8 @@
     "    return (x, y)\n",
     "\n",
     "\n",
-    "loader._map_fns = [process_batch]"
+    "loader = loader.map(process_batch)\n",
+    "loader.stop()"
    ]
   },
   {
@@ -414,8 +414,7 @@
     }
    ],
    "source": [
-    "batch = next(iter(loader))\n",
-    "batch"
+    "loader.peek()"
    ]
   },
   {
@@ -436,8 +435,8 @@
     "class MatrixFactorization(tf.keras.Model):\n",
     "    def __init__(self, n_factors):\n",
     "        super().__init__()\n",
-    "        self.user_embeddings = tf.keras.layers.Embedding(ratings['userId'].max(), n_factors)\n",
-    "        self.movie_embeddings = tf.keras.layers.Embedding(ratings['movieId'].max(), n_factors)\n",
+    "        self.user_embeddings = tf.keras.layers.Embedding(ratings['userId'].max() + 1, n_factors)\n",
+    "        self.movie_embeddings = tf.keras.layers.Embedding(ratings['movieId'].max() + 1, n_factors)\n",
     "\n",
     "    def call(self, batch, training=False):\n",
     "        user_embs = self.user_embeddings(batch['userId'])\n",
diff --git a/examples/01b-Getting-started-Pytorch.ipynb b/examples/01b-Getting-started-Pytorch.ipynb
index 1728b574..d290ed7e 100644
--- a/examples/01b-Getting-started-Pytorch.ipynb
+++ b/examples/01b-Getting-started-Pytorch.ipynb
@@ -292,8 +292,7 @@
     }
    ],
    "source": [
-    "batch = loader.peek()\n",
-    "batch"
+    "loader.peek()"
    ]
   },
   {
@@ -335,7 +334,9 @@
     "optim = Adam\n",
     "weight_decay = 0\n",
     "\n",
-    "model = MatrixFactorization(64).cuda()\n",
+    "device = \"cuda\" if torch.cuda.is_available() else \"cpu\"\n",
+    "\n",
+    "model = MatrixFactorization(64).to(device)\n",
     "optimizer = optim(model.parameters(), lr=lr, weight_decay=weight_decay)\n",
     "criterion = nn.MSELoss()"
    ]
diff --git a/tox.ini b/tox.ini
index b5a84445..00ee19fb 100644
--- a/tox.ini
+++ b/tox.ini
@@ -20,7 +20,7 @@ commands =
     python -m pip install --upgrade git+https://github.com/NVIDIA-Merlin/nvtabular.git@{posargs:main}
     python -m pip install -e .[all]
     python -m pip install fsspec==2022.5.0
-    python -m pytest --cov-report term --cov=merlin -rxs tests/unit
+    python -m pytest --cov-report term --cov=merlin -rxs tests/unit tests/examples
 
 [testenv:test-tensorflow]
 ; Runs in: Github Actions
@@ -33,7 +33,7 @@ commands =
     python -m pip install --upgrade git+https://github.com/NVIDIA-Merlin/nvtabular.git@{posargs:main}
     python -m pip install -e .[tensorflow]
     python -m pip install fsspec==2022.5.0
-    python -m pytest -rxs -m tensorflow tests/unit
+    python -m pytest -rxs -m tensorflow tests/unit tests/examples
 
 [testenv:test-torch]
 ; Runs in: Github Actions
@@ -46,7 +46,7 @@ commands =
     python -m pip install --upgrade git+https://github.com/NVIDIA-Merlin/nvtabular.git@{posargs:main}
     python -m pip install -e .[torch]
     python -m pip install fsspec==2022.5.0
-    python -m pytest -rxs -m torch tests/unit
+    python -m pytest -rxs -m torch tests/unit tests/examples
 
 [testenv:test-jax]
 ; Runs in: Github Actions
@@ -59,7 +59,7 @@ commands =
     python -m pip install --upgrade git+https://github.com/NVIDIA-Merlin/nvtabular.git@{posargs:main}
     python -m pip install -e .[jax]
     python -m pip install fsspec==2022.5.0
-    python -m pytest -rxs -m jax tests/unit
+    python -m pytest -rxs -m jax tests/unit tests/examples
 
 [testenv:test-gpu]
 passenv =
@@ -79,7 +79,7 @@ commands =
     python -m pip install --upgrade git+https://github.com/NVIDIA-Merlin/core.git
     python -m pip install --upgrade git+https://github.com/NVIDIA-Merlin/nvtabular.git
 
-    python -m pytest --cov-report term --cov merlin -rxs tests/unit
+    python -m pytest --cov-report term --cov merlin -rxs tests/unit tests/examples
 
 [testenv:docs]
 ; Runs in: Github Actions


Test patch:
diff --git a/ci/test_unit.sh b/ci/test_unit.sh
new file mode 100644
index 00000000..e840d568
--- /dev/null
+++ b/ci/test_unit.sh
@@ -0,0 +1,20 @@
+#
+# Copyright (c) 2022, NVIDIA CORPORATION.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+#!/bin/bash
+set -e
+
+pytest -rxs tests/unit tests/examples
diff --git a/tests/examples/test_getting_started_pytorch.py b/tests/examples/test_getting_started_pytorch.py
index 438d7bee..9c98bf41 100644
--- a/tests/examples/test_getting_started_pytorch.py
+++ b/tests/examples/test_getting_started_pytorch.py
@@ -6,7 +6,7 @@
 pytestmark = pytest.mark.tensorflow
 
 
-@testbook("examples/01b-Getting-started-Pytorch", execute=False)
+@testbook("examples/01b-Getting-started-Pytorch.ipynb", execute=False)
 def test_getting_started_pytorch(tb):
     tb.inject(
         """