Fix/dataset index: Index values were faulty when indexing the original samples instead of blocks.

[le1nux]

What does this PR do?

The index values in the pbin files had the wrong values. They did start with an offset and additionally, we added another offset of HEADER size when reading from the file buffer.
See here for the initial offset during pbin index creation:

modalities/src/modalities/dataloader/create_packed_data.py

Line 145 in 4aa2e88

curr_offset = EmbeddedStreamData.HEADER_SIZE_IN_BYTES

and the additional offset that is used when reading from the memmap during training:

modalities/src/modalities/dataloader/create_packed_data.py

Line 262 in 4aa2e88

self.data = np.memmap(self._data_path, mode="r", offset=self.HEADER_SIZE_IN_BYTES, shape=(self.data_len,))

This PR fixes this issue and makes the index always start at byte 0, only applying the offset once when reading from the memmap file.

General changes

• index tuples are now always in bytes and the start of the first sample in the data section starts at byte 0 (before the was a wrong offset)
• added test for indexing the original samples
• the documentation was updated accordingly
• removed block_size from abstract classes that don't need to see the block_size concept

Breaking Changes

• None

Checklist before submitting final PR

• My PR is minimal and addresses one issue / enhancement in isolation
• I have merged main into this feature branch
• I have reviewed my own code w.r.t. correct implementation, missing type hints, proper documentation, etc.
• I have run a sample config for model training
• I have fixed all failing tests (python tests/tests.py)

[le1nux]

fixes #163

[flxst]

Nice work! I have two general remarks:

1. I think that the class inheritance structure, PackedMemMapDatasetBase -> PackedMemMapDatasetBase, PackedMemMapDatasetContinuous is suboptimal now that the parent class PackedMemMapDatasetBase is not an abstract base class anymore, but actually can be instantiated and used by itself. It might be cleaner to restore an abstract base class with an abstract method _generate_packing_index, and inherit three classes from it with the three different implementations of the method.
2. The names of the classes are hard to understand and maybe a bit misleading. For instance, the class PackedMemMapDatasetBase does not actually do packing, does it? Also, why Continuous? Maybe we could try to find better names.

I will approve the PR now since I don't believe that my above suggestions for improvement are essential, but I am happy to take another look if you decide to implement them!

[flxst]

I do not like that this method is overwritten in the inherited classes PackedMemMapDatasetContinuous and PackedMemMapDatasetMegatron, see my general comment.

[le1nux]

will be adressed as part of a new PR and issue #167

[le1nux]

Yes, the inheritance structure can be improved. I suggest we do this in a separate PR together with improving the "packing" terms in those cases when there is no actual packing happening.

I added the issue #167 for addressing this.