Fixes for several issues: mask compile, cache offload, buffers, FSDP2 eval, trainer teardown, MoE logits, tokenizers, processors - #48971
Closed
Ryukijano wants to merge 16 commits into
Conversation
…uggingfaceGH-48924] [huggingfaceGH-48925] Signed-off-by: Gyanateet <gyanateet@gmail.com>
…yers [huggingfaceGH-48947] Signed-off-by: Gyanateet <gyanateet@gmail.com>
…ts does not cover them [huggingfaceGH-48892] Signed-off-by: Gyanateet <gyanateet@gmail.com>
…gingfaceGH-48841] Signed-off-by: Gyanateet <gyanateet@gmail.com>
Signed-off-by: Gyanateet <gyanateet@gmail.com>
…H-48527] Signed-off-by: Gyanateet <gyanateet@gmail.com>
Signed-off-by: Gyanateet <gyanateet@gmail.com>
…ingfaceGH-48225] Signed-off-by: Gyanateet <gyanateet@gmail.com>
…ngfaceGH-48225] Signed-off-by: Gyanateet <gyanateet@gmail.com>
…uggingfaceGH-48818] Signed-off-by: Gyanateet <gyanateet@gmail.com>
…OutputWithPast docs [huggingfaceGH-48886] [huggingfaceGH-48889] Signed-off-by: Gyanateet <gyanateet@gmail.com>
…gfaceGH-48756] Signed-off-by: Gyanateet <gyanateet@gmail.com>
…process group [huggingfaceGH-48874] Signed-off-by: Gyanateet <gyanateet@gmail.com>
huggingfaceGH-48942] Signed-off-by: Gyanateet <gyanateet@gmail.com>
…els [huggingfaceGH-48917] Signed-off-by: Gyanateet <gyanateet@gmail.com>
huggingfaceGH-48862] Signed-off-by: Gyanateet <gyanateet@gmail.com>
Contributor
|
Thank you for your contribution 🤗! CI Security Gate — automatic approval blockedThis PR was not automatically approved for CI because the security gate failed. Possible reasons:
See the workflow run for the exact violations. A maintainer can review and manually approve CI if a finding is a false positive. |
Contributor
|
[For maintainers] Suggested jobs to run (before merge) run-slow: afmoe, albert, bark, camembert, clipseg, cohere2_moe, cohere_compass, ernie4_5_vl_moe, gemma4, glm_image, hunyuan_vl, lasr, luke, mbart50, mellum, minimax_m3_vl |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR bundles a set of small, independent fixes for several open issues. Each fix is a separate signed-off commit with its own regression coverage, so the PR can be split by subsystem if preferred.
Mask handling under
torch.compile(#48924, #48925)create_causal_mask/ bidirectional mask helpers now distinguish ordinarytorch.compile(whereis_tracing(None)is true but aNonepadding mask still permits skipping mask construction) from export/JIT/CUDA-graph capture, which genuinely requires static behavior.modeling_optno longer forwards the fabricated all-ones position-ID mask tocreate_causal_mask, preserving theNonefast path.sdpa_maskreturnsNoneunder compile; compiled OPT forward matches eager (max diff ~1.2e-7).Hybrid cache offloading of linear-attention states (#48947)
Cache.updatesynchronizes the offload prefetch stream only for KV updates. Hybrid layers also holdconv_states/recurrent_statesthat are updated throughupdate_conv_state/update_recurrent_stateand read directly by model code, so offloading them races and produces CUDA/CPU device mismatches (reproduced intorch.catduring decode).LinearAttentionAnd*Layer.offload()/.prefetch()now manage only the dynamic KV state; missing implementations were added to the sliding hybrid layers, andCache.is_linearexcludes all hybrid classes.tests/utils/test_cache_utils.py.Non-persistent buffer materialization (#48892)
_move_missing_keys_from_meta_to_devicematerializes non-persistent buffers withtorch.zeros_likeinstead oftorch.empty_like(garbage memory), and warns once for non-empty buffers that remain all-zero after initialization, i.e. not covered by_init_weights. Rotaryinv_freqbuffers do not false-positive.FSDP2 evaluation before training (#48841)
evaluation_loopnow creates the optimizer and callsaccelerator.prepare(model, optimizer)when FSDP2 is active and the model has not been prepared yet, matching the training path so FSDP2 rewrites optimizer parameter references consistently. (Supersedes the approach in the closed Fix FSDP2 evaluation before training #48858.)Trainerteardown / process-group lifecycle (#48874)Trainer.end()(idempotent) callsaccelerator.end_training(), which finishes trackers and destroys the distributed process group.close()is an alias;Traineris now a context manager (with Trainer(...) as trainer:).atexitfallback destroys the process group at interpreter exit only when accelerate owns it (a user-initializedtorch.distributedgroup withoutPartialStateis left alone). The group is deliberately not destroyed in_finalize_trainingsinceevaluate/predict/save_model/push_to_hubstill need it._no_placement_paramsin balanced memory (#48756)get_balanced_memoryacceptsno_placement_paramsand subtracts excluded-parameter sizes from the total, the holding no-split module, and the holding leaf module — so a ~100 GB never-placed table (e.g. Qwen4-Exp's n-gram embedding) no longer inflates every device's budget. Adapted to this branch's newerbiggest_leafbuffer logic; regression test added intest_modeling_utils.py.base_model_prefixcorrections (#48862)rename_source_keynow requiresprefix + "."when stripping (prevents matchingmodel2.for prefixmodel) and escapes the prefix in the regex.clip, PPOCRv5/v6 det →model, TimesFM →decoder, UVDoc →backbone, VideoPrism →vision_model/text_model/video_model.MoE router logits / aux loss (#48886, #48889)
cohere2_moegains theoutput_router_logitsconfig flag + forward plumbing its code already honoured.afmoe/zaya(propagated) return explicitaux_loss=None: these models balance experts with per-expert bias (expert_bias,e_score_correction_bias,balancing_biases), so no aux loss is computed —router_logitsare still returned for inspection/external balancing. Wiring a Switch-style loss would also crash zaya (num_experts+1router logits incl. skip expert).MoeCausalLMOutputWithPastdocstring updated accordingly. This intentionally matches the approach onorigin/moe-router-logits-exposedrather than adding losses these recipes don't use.Gemma4 audio eager mask (#48818)
Gemma4AudioAttentiontreated the 4-D float additive mask (0 = keep,finfo.min= masked) as a boolean keep-mask and inverted it;_convert_4d_mask_to_blocked_5dalso padded float masks withFalse(= 0.0 = keep). Float masks are now normalized to boolean keep-masks (mask == 0). Eager vs SDPA max diff: 0.358 → 0.0.Image/video processor
sizedict mutation (#48917)size["min_pixels"]/["max_pixels"]now copy the dict first (size = dict(size) if isinstance(size, dict) else size), so class-level defaults and caller-passed dicts are no longer mutated. Applied to qwen2_vl (image ×2 + video), minimax_m3_vl (incl. the[672,672]legacy path), ernie4_5_vl_moe video, and all modular-derived processors (video_llama_3, paddleocr_vl, hunyuan_vl, glm_image, cohere_compass). Unlike Fix image processor class-level size mutation and min_pixels handling #48916 this guards non-dictsizeinputs too (int/list still valid perImagesKwargs).Empty SentencePiece
precompiled_charsmap(#48942)b""for unset bytes fields, sob""— notNone— reached everyif _spm_precompiled_charsmap is not None:guard andnormalizers.Precompiled(b"")crashed. Guards switched to truthiness in 14 tokenizers (t5, xlnet, xlm_roberta, xglm, videoprism, rembert, reformer, pegasus, nllb, mbart50, lasr [modular+generated], camembert, albert).convert_slow_tokenizer.pyalready used truthiness.Luke/MLuke entity tokens (#48225)
entity_token_1/entity_token_2IDs are resolved by token name viaconvert_tokens_to_idsinstead of positionalextra_special_tokens_ids[0]/[1], which broke whenever other extra special tokens occupied those positions. Applied to bothmlukeandluke(same bug).Chat CLI
requestsdependency (#48283)transformers.cli.chatno longer hard-requiresrequests: streaming now useshuggingface_hub.utils.httpx(already a transitive dep) with a context-managedhttpx.stream, preserving indefinite streaming and error handling.transformers env/versionverified working withrequestsblocked.Test robustness
@is_flaky(max_attempts=2)(Flaky MellumModelTest.test_load_balancing_loss — the only copy without @is_flaky #48138).Notes on overlap with existing PRs
Several issues have open competing PRs (#48916/#48921, #48871, #48875, #48758, #48958, #48928–#48940, #48932, #48922, #48318/#48412, #48389, #48530/#48543/#48587/#48944, #48226/#48230/#48235/#48408). Where they exist this implementation generally differs (e.g. guarded dict copy vs unconditional,
finfo.min-aware mask normalization, escaped-prefix regex fix, upstream-aligned MoE approach, ownership-gated atexit teardown). Happy to split this into per-issue PRs or drop commits where a competing approach is preferred — tell me which direction you want.Testing
ruff checkandruff format --checkclean on all 68 changed files.