fix: use ceil for minimum num_files_train (rule 3.1.2)#796
Merged
FileSystemGuy merged 1 commit intoJul 15, 2026
Merged
Conversation
…3.1.2 The memory-derived minimum (5x host memory / record_length / samples_per_file) is a float. Datagen and the runtime verifier truncated to floor via `//` and `int()`, so a dataset sized to the floor passed at runtime. Rule 3.1.2 in the submission checker then re-derived the threshold as a float and rejected the same dataset by a single file, with a confusing "N < N" message because the error also `int()`-truncated the float for display. Compute both min_num_files_by_bytes and min_num_files_by_samples with math.ceil in rules/utils.py so datasize/dry-run and the runtime verifier report the ceil. Compute min_total_files with math.ceil in the 3.1.2 check so the comparison is int-vs-int and the violation message shows the true required count.
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
This was referenced Jul 15, 2026
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
1 participant
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.
Summary
5 × host_memory / record_length / samples_per_file) is a float. Datagen and the runtime verifier were truncating to floor via//andint(), so a dataset sized to the floor passed at runtime but the post-submission checker re-derived the threshold as a float and rejected the same dataset by a single file.int()-truncated the float, producing a confusing"4710038 < 4710038".math.ceilformin_num_files_by_bytesandmin_num_files_by_samplesinrules/utils.py, and formin_total_filesin the 3.1.2 check. All three sites (datasize/dry-run, runtime verifier, submission checker) now agree.Note
This is a retroactive review PR. Commit
f9d414dwas already pushed tomain(bypassing the PR requirement). The base of this PR (review-base/pre-ceil-fix) points at90bc58e, the parent of the fix, so the diff is viewable for review.Test plan
uv run pytest tests— 2913 passeduv run pytest mlpstorage_py/tests— 853 passeduv run pytest vdb_benchmark/tests— 174 passeduv run pytest kv_cache_benchmark/tests— 238 passed