facebookresearch / facebookresearch/fairseq2

Integrity check for tokenizer downloads uses the wrong checksum field

Open
#865 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Python
Stars
1.1k
Forks
144
Avg merge
4d 1h
Merged PRs (30d)
1

Description

**Describe the bug:**
Recently, #840 was merged that allows the download manager to validate models, datasets, and tokenizers with an MD5 hash after completion. Because tokenizers and models share the same assert card for metadata, they require 2 separate checksum fields to distinguish the two assets but currently both default to the same ``checksum`` field.

**Describe how to reproduce:**

```python
import fairseq2
from fairseq2.assets import default_asset_store, InProcAssetDownloadManager

fairseq2.setup_fairseq2()
card = default_asset_store.retrieve_card("mistral_7b")
uri = card.field("tokenizer").as_uri() # swap with "checkpoint"
checksum = card.field("checksum").get_as_(str)
download_manager = InProcAssetDownloadManager()

download_manager.download_tokenizer(uri, model_name="mistral", checksum=checksum) # swap with download_checkpoint
```
The above code sample more or less behaves like how the download manager is used to download the tokenizer. The real issue lies with the fact that [text_tokenizer.py](https://github.com/facebookresearch/fairseq2/blob/2ab824540c366925df447f5fc95377b698b41e08/src/fairseq2/data/text/text_tokenizer.py#L226) uses the same ``checksum`` field as [loader.py](https://github.com/facebookresearch/fairseq2/blob/2ab824540c366925df447f5fc95377b698b41e08/src/fairseq2/models/loader.py#L278) when it really should use something different like ``tokenizer_checksum``.

**Describe the expected behavior:**
Both assets should be validated with separate hashes because they are different assets. This should be easily resolved by changing [text_tokenizer.py](https://github.com/facebookresearch/fairseq2/blob/2ab824540c366925df447f5fc95377b698b41e08/src/fairseq2/data/text/text_tokenizer.py#L226) to look for a separate field like ``tokenizer_checksum``.

**Environment:**
fairseq2: 0.3.0.dev0
PyTorch: 2.4.0+cu121
Python: 3.10.12
OS: Windows 10 (WSL)

**Additional Context:**
None

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.