deepseek-ai/DeepSeek-Prover-V1.5

Fork 0

mirror of https://github.com/deepseek-ai/DeepSeek-Prover-V1.5.git synced 2026-11-05 00:57:20 +00:00

[PR #25] Restrict each GeneratorProcess to a single visible GPU #17

New issue

Open

opened 2026-10-26 21:06:40 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 21:06:40 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/deepseek-ai/DeepSeek-Prover-V1.5/pull/25
Author: @karandhaodiyal28-hash
Created: 8/9/2026
Status: 🔄 Open

Base: main ← Head: fix/multi-gpu-device-placement

📝 Commits (1)

d2ccb18 Restrict each GeneratorProcess to a single visible GPU

📊 Changes

1 file changed (+7 additions, -0 deletions)

View changed files

📝 prover/workers/generator.py (+7 -0)

📄 Description

What does this PR do?

Fixes #47612

Loading google/siglip2-base-patch16-naflex warns that bos_token_id=49406 / eos_token_id=49407 are "not within the vocabulary (between 0 and 31999)" even though the checkpoint's real vocab is 256000. The warning comes from throwaway default config instances built internally (e.g. by to_diff_dict), which @strict validation inspects — and the class defaults of SiglipTextConfig are indeed invalid: 49406/49407 are CLIP leftovers, while no SigLIP checkpoint uses a BOS token and EOS resolves to id 1 in the reference tokenizers.

This directly follows the maintainer feedback from the review of #47613, where @zucchini-nlp and @vasqu ruled out changing logging verbosity and preferred correcting the wrong defaults instead. Concretely, this PR:

sets bos_token_id=None and eos_token_id=1 in SiglipTextConfig
regenerates configuration_siglip2.py so Siglip2TextConfig picks up the same defaults via modular
keeps Aimv2TextConfig.eos_token_id explicitly at 49407 (AIMv2 uses the CLIP tokenizer, vocab_size=49408, where that value is correct — it previously inherited it silently from SiglipTextConfig)
adds a regression test asserting no out-of-vocabulary warning is emitted when building a default config and calling to_diff_dict

Where `eos_token_id=1` and `bos_token_id=None` come from

Read off the tokenizer files of the reference checkpoints on the Hub:

google/siglip-base-patch16-224/tokenizer_config.json: added_tokens_decoder maps id 1 to </s>, with "eos_token": "</s>" and "pad_token": "</s>" — there is no BOS token at all. Note pad and EOS sharing id 1 is the reference tokenizer's own behavior (it pads with </s>), which is also why the existing pad_token_id=1 default stays untouched; this PR doesn't introduce the pad == eos overlap, it matches it.
google/siglip2-base-patch16-naflex/tokenizer_config.json (Gemma tokenizer): added_tokens_decoder maps id 1 to <eos> (<pad> is 0, <bos> is 2), with "add_bos_token": false, "add_eos_token": true — so EOS is id 1 here as well, and BOS is never added.

Nothing in modeling_siglip*.py reads these config fields, and the google/siglip* checkpoints on the Hub don't store them, so no stored checkpoint value is overridden. The other models inheriting from SiglipTextConfig (aimv2, videoprism, tipsv2) either override the defaults themselves or, in the case of aimv2's eos_token_id, are now pinned explicitly to their previous value.

Verified locally (CPU):

the repro from #47612 is silent and reports vocab_size=256000, bos=None, eos=1
pytest tests/models/siglip tests/models/siglip2 — 772 passed
utils/check_modular_conversion.py on siglip2, aimv2, videoprism, tipsv2 — clean
utils/check_config_attributes.py — clean
ruff check / ruff format --check on the touched files — clean

Who can review?

@zucchini-nlp @vasqu (per the discussion in #47613)

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/deepseek-ai/DeepSeek-Prover-V1.5/pull/25 **Author:** [@karandhaodiyal28-hash](https://github.com/karandhaodiyal28-hash) **Created:** 8/9/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/multi-gpu-device-placement` --- ### 📝 Commits (1) - [`d2ccb18`](https://github.com/deepseek-ai/DeepSeek-Prover-V1.5/commit/d2ccb187b779b601d8344361b386c433cd965c96) Restrict each GeneratorProcess to a single visible GPU ### 📊 Changes **1 file changed** (+7 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `prover/workers/generator.py` (+7 -0) </details> ### 📄 Description # What does this PR do? Fixes #47612 Loading `google/siglip2-base-patch16-naflex` warns that `bos_token_id=49406` / `eos_token_id=49407` are "not within the vocabulary (between 0 and 31999)" even though the checkpoint's real vocab is 256000. The warning comes from throwaway default config instances built internally (e.g. by `to_diff_dict`), which `@strict` validation inspects — and the *class defaults* of `SiglipTextConfig` are indeed invalid: 49406/49407 are CLIP leftovers, while no SigLIP checkpoint uses a BOS token and EOS resolves to id 1 in the reference tokenizers. This directly follows the maintainer feedback from the review of #47613, where @zucchini-nlp and @vasqu ruled out changing logging verbosity and preferred correcting the wrong defaults instead. Concretely, this PR: - sets `bos_token_id=None` and `eos_token_id=1` in `SiglipTextConfig` - regenerates `configuration_siglip2.py` so `Siglip2TextConfig` picks up the same defaults via modular - keeps `Aimv2TextConfig.eos_token_id` explicitly at `49407` (AIMv2 uses the CLIP tokenizer, `vocab_size=49408`, where that value is correct — it previously inherited it silently from `SiglipTextConfig`) - adds a regression test asserting no out-of-vocabulary warning is emitted when building a default config and calling `to_diff_dict` ## Where `eos_token_id=1` and `bos_token_id=None` come from Read off the tokenizer files of the reference checkpoints on the Hub: - [`google/siglip-base-patch16-224/tokenizer_config.json`](https://huggingface.co/google/siglip-base-patch16-224/raw/main/tokenizer_config.json): `added_tokens_decoder` maps id `1` to `</s>`, with `"eos_token": "</s>"` and `"pad_token": "</s>"` — there is no BOS token at all. Note pad and EOS sharing id 1 is the reference tokenizer's own behavior (it pads with `</s>`), which is also why the existing `pad_token_id=1` default stays untouched; this PR doesn't introduce the pad == eos overlap, it matches it. - [`google/siglip2-base-patch16-naflex/tokenizer_config.json`](https://huggingface.co/google/siglip2-base-patch16-naflex/raw/main/tokenizer_config.json) (Gemma tokenizer): `added_tokens_decoder` maps id `1` to `<eos>` (`<pad>` is 0, `<bos>` is 2), with `"add_bos_token": false, "add_eos_token": true` — so EOS is id 1 here as well, and BOS is never added. Nothing in `modeling_siglip*.py` reads these config fields, and the `google/siglip*` checkpoints on the Hub don't store them, so no stored checkpoint value is overridden. The other models inheriting from `SiglipTextConfig` (aimv2, videoprism, tipsv2) either override the defaults themselves or, in the case of aimv2's `eos_token_id`, are now pinned explicitly to their previous value. Verified locally (CPU): - the repro from #47612 is silent and reports `vocab_size=256000`, `bos=None`, `eos=1` - `pytest tests/models/siglip tests/models/siglip2` — 772 passed - `utils/check_modular_conversion.py` on siglip2, aimv2, videoprism, tipsv2 — clean - `utils/check_config_attributes.py` — clean - `ruff check` / `ruff format --check` on the touched files — clean ## Who can review? @zucchini-nlp @vasqu (per the discussion in #47613) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-26 21:06:40 +00:00

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

deepseek-ai/DeepSeek-Prover-V1.5#17

No description provided.

Rows
Columns