Conversation
The doc comment summarized the preset in one prose sentence. List all twelve fields with their default and high performance values, the memory-derived buffer formulas, the worst-case buffer allocation, and the fact that each field can also be set individually via its own HF_XET_* variable. Fixes huggingface#926 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Collaborator
|
There's no need to repeat the code in the comment. |
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.
Fixes #926.
Answering the scoping question in the issue thread: this PR takes the xet-core code comment half. The hub docs describe each
HF_XET_*variable individually, but the doc comment onwith_high_performance()is the one place that defines what the preset itself does, and it summarized twelve assignments in one sentence with no memory figure.Since #943 the buffer values are memory-derived, so the comment now lists:
usable/16 -> usable/8etc.) with floors/ceilings, plus the static fallbacks used when memory cannot be probed;download_buffer_size + n_active * download_buffer_perfile_sizecapped atdownload_buffer_limit(verified againstFileReconstructor's target formula) — up to 32GB at the ceilings with the default 8 concurrent downloads;data.max_concurrent_file_downloadsis not raised by the preset, only its ingestion twin;HF_XET_*variable, so parts of the preset can be adopted individually on memory-constrained machines.All values checked against the field defaults in
config/groups/and the constants inutils/system_memory.rson current main.Comment-only diff; no Rust toolchain on this machine so I did not run cargo locally, but no code is touched and the
[floor, ceiling]notation matches the existing field docs inreconstruction.rs.Written with Claude Code, values verified against the source as described above.
Note
Low Risk
Comment-only change with no code or configuration logic modified.
Overview
Expands the
with_high_performance()doc comment so the high-performance preset is documented in one place instead of a single vague sentence.The comment now lists twelve fields with default → preset values (ingestion concurrency, adaptive concurrency bounds, reconstruction fetch sizes, and memory-derived download buffer formulas with clamps). It explains how usable memory is computed, static 16GB / 2GB / 64GB fallbacks when probing is disabled or fails, and the worst-case download buffer formula (up to ~32GB at ceilings with default concurrent downloads). It also notes that
max_concurrent_file_downloadsis not raised by the preset, and that each field can be tuned via its ownHF_XET_*env var without enabling the full preset.No runtime behavior changes — documentation only on
xet_config.rs.Reviewed by Cursor Bugbot for commit 49fb4e8. Bugbot is set up for automated code reviews on this repo. Configure here.