diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 2eea7332252..d0aeb20875b 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -91,7 +91,29 @@ jobs: - uses: actions/setup-python@v6 with: python-version: "3.12" + - name: Record which CPU this runner got + # The 0xc000001d crash this job used to hit tracked the host CPU, not the test, so this + # is the first thing worth knowing if it ever recurs. See the ONEDNN_MAX_CPU_ISA comment + # on the test step below. + continue-on-error: true + shell: pwsh + run: | + Get-CimInstance Win32_Processor | Select-Object Name, Description, NumberOfCores | Format-List + python -c "import platform; print('machine:', platform.machine(), platform.processor())" - name: Run unit tests (without coverage) + env: + # Cap oneDNN's instruction set. bf16 linear/matmul on CPU dispatch through oneDNN, + # which by default picks the highest ISA the host advertises -- Intel AMX on the + # Emerald Rapids runners in the Actions fleet. On those hosts that path executes an + # instruction that faults with #UD, which surfaces as 0xc000001d and kills the whole + # pytest process. AMD EPYC runners have neither AMX nor AVX-512, take the AVX2 path, + # and have never shown the crash -- so AVX2 is the ceiling with evidence behind it. + # This is a CI mitigation for a torch/oneDNN Windows issue, not a ModelOpt bug: it + # changes which kernel runs, not what is being tested. To check whether it is still + # needed, drop it and run the suite repeatedly until an Intel host comes up. + # See the PR that added this for the six crashes and the host correlation behind it. + ONEDNN_MAX_CPU_ISA: "AVX2" + DNNL_MAX_CPU_ISA: "AVX2" run: pip install nox uv && nox -s "unit-3.12(torch_214, tf_latest)" multi-version: if: needs.check-file-changes.outputs.any_changed == 'true' @@ -201,9 +223,14 @@ jobs: runs-on: ubuntu-latest steps: - name: Required unit tests did not succeed + # windows is deliberately absent from this condition. It still runs and is still visible, + # but it does not gate the merge: the job is flaky for reasons unrelated to the change + # under review -- an MSVC extension build racing the per-test timeout, and an + # 0xc000001d illegal instruction that depends on which CPU the runner draws. What we + # actually care about on that platform is the ONNX surface, so blocking every PR on + # unrelated torch flakiness costs more than it catches. if: >- ${{ needs.linux.result != 'success' || (needs.check-file-changes.outputs.any_changed == 'true' && ( - needs.windows.result != 'success' || needs.multi-version.result != 'success' || needs.partial-install.result != 'success' || needs.launcher.result != 'success' || diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c15cd53075d..15eb3b12cc7 100755 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -73,6 +73,7 @@ Changelog - Fix a DDP hang in DFlash training at scale where a rank whose batch contained no valid anchor skipped the draft forward, leaving its rotary buffer list shorter than other ranks' and causing ``broadcast_buffers`` to hang. The buffer is now created during ``modify()`` before training begins. - Fix ``megatron_generate`` dropping the VLM vision inputs (``pixel_values`` / ``image_grid_thw`` / ``image_sizes``) after the first generated token when KV-cache decoding is off, including the automatic fallback under sequence parallelism, which made generation silently ignore the image. No other ModelOpt feature is affected. - Fix two issues in the vLLM offline hidden-state dump (``examples/speculative_decoding/collect_hidden_states/compute_hidden_states_vllm.py``) that only surface on large runs. **Resume:** the filter that skips conversations whose ``.pt`` already exists now runs with ``load_from_cache_file=False``. It depends on on-disk state, which is not part of the fingerprint ``datasets`` computes from the function and the dataset, so with a persistent HF cache reused across a resumed or requeued run the cached "keep everything" result from an earlier run was replayed and the dump re-generated and overwrote conversations it had already finished (observed: tens of thousands of ``.pt`` rewritten while the output count stayed flat). **Staging:** generation is now chunked (``--save-chunk-size``, default 256), so each chunk is saved and its staged hidden states freed before the next chunk is generated. Previously the whole dataset was generated before anything was saved, which kept every conversation staged in the connector's ``shared_storage_path`` (``/dev/shm``, i.e. RAM, by default) at once and exhausted it partway through large dumps. Chunking also makes the dump incrementally durable, so an interrupted run keeps its finished conversations and resumes from them. The save path now also frees each conversation's staged hidden states in a ``finally``, so a conversation skipped mid-loop (e.g. a short ``loss_mask``) can no longer leak its staging file, and conversation ids are validated as plain filenames before being used to build output paths. +- Fix YAML config I/O decoding with the locale codepage instead of UTF-8, which made a config containing any non-ASCII byte fail to load on a machine whose locale is not UTF-8 (notably Windows, where the default is cp1252). ``modelopt/recipe/loader.py``, the two ONNX autotune state files, the two transformers config readers, the distill config and the puzzletron profile now pass ``encoding="utf-8"`` explicitly. Only the YAML config paths are covered: these are the files most likely to carry non-ASCII text in comments, model names or paths, and the only ones read inside a user's process. 0.47.0 (2026-09-xx) ^^^^^^^^^^^^^^^^^^^ diff --git a/modelopt/onnx/quantization/autotune/autotuner_base.py b/modelopt/onnx/quantization/autotune/autotuner_base.py index 22de4900435..0c828fe9ce5 100644 --- a/modelopt/onnx/quantization/autotune/autotuner_base.py +++ b/modelopt/onnx/quantization/autotune/autotuner_base.py @@ -737,7 +737,7 @@ def save_state(self, output_path: str) -> None: "patterns": [pattern_schemes.to_dict() for pattern_schemes in self.profiled_patterns], } - with open(output_path, "w") as f: + with open(output_path, "w", encoding="utf-8") as f: yaml.dump(state, f, default_flow_style=False, sort_keys=False) num_patterns = len(self.profiled_patterns) @@ -775,7 +775,7 @@ def load_state(self, input_path: str) -> None: AutotunerNotInitializedError: If initialize() hasn't been called FileNotFoundError: If the input_path doesn't exist """ - with open(input_path) as f: + with open(input_path, encoding="utf-8") as f: state = yaml.safe_load(f) if state.get("baseline_latency_ms") is not None: diff --git a/modelopt/onnx/quantization/autotune/common.py b/modelopt/onnx/quantization/autotune/common.py index 31983423cd9..a0aa6744cca 100644 --- a/modelopt/onnx/quantization/autotune/common.py +++ b/modelopt/onnx/quantization/autotune/common.py @@ -739,7 +739,7 @@ def save(self, output_path: str) -> None: """ state = self.to_dict() - with open(output_path, "w") as f: + with open(output_path, "w", encoding="utf-8") as f: yaml.dump(state, f, default_flow_style=False, sort_keys=False) logger.info( @@ -768,7 +768,7 @@ def load(cls, input_path: str) -> "PatternCache": Raises: FileNotFoundError: If the input_path doesn't exist """ - with open(input_path) as f: + with open(input_path, encoding="utf-8") as f: state = yaml.safe_load(f) cache = cls.from_dict(state) diff --git a/modelopt/recipe/loader.py b/modelopt/recipe/loader.py index 91e2bac75de..7f8591b0123 100644 --- a/modelopt/recipe/loader.py +++ b/modelopt/recipe/loader.py @@ -171,7 +171,7 @@ def _peek_recipe_type(recipe_file: Path | Traversable) -> RecipeType | None: import yaml try: - raw = yaml.safe_load(recipe_file.read_text()) + raw = yaml.safe_load(recipe_file.read_text(encoding="utf-8")) return RecipeType(raw["metadata"]["recipe_type"]) except (TypeError, KeyError, ValueError): return None @@ -201,7 +201,7 @@ def _load_recipe_from_file( if required_section is not None: import yaml - raw = yaml.safe_load(recipe_file.read_text()) or {} + raw = yaml.safe_load(recipe_file.read_text(encoding="utf-8")) or {} if not isinstance(raw, dict) or required_section not in raw: # Strip only the ``speculative_`` prefix so multi-word non-speculative types # (e.g. ``auto_quantize``) keep their full name: AUTO_QUANTIZE, not QUANTIZE. diff --git a/modelopt/torch/distill/plugins/megatron.py b/modelopt/torch/distill/plugins/megatron.py index c93f0961d1f..581f670759c 100644 --- a/modelopt/torch/distill/plugins/megatron.py +++ b/modelopt/torch/distill/plugins/megatron.py @@ -120,7 +120,7 @@ def setup_distillation_config( elif isinstance(config_or_path, DistillationConfig): cfg = config_or_path else: - with open(config_or_path) as f: + with open(config_or_path, encoding="utf-8") as f: cfg = yaml.safe_load(f) cfg = DistillationConfig(**cfg) diff --git a/modelopt/torch/opt/plugins/transformers.py b/modelopt/torch/opt/plugins/transformers.py index a291b5abf36..6665fdcaedc 100644 --- a/modelopt/torch/opt/plugins/transformers.py +++ b/modelopt/torch/opt/plugins/transformers.py @@ -344,7 +344,7 @@ def parse_args_into_dataclasses(self, args=None, **kwargs): args = args[:idx] + args[idx + 2 :] # strip --config from argv import yaml - with open(config_path) as f: + with open(config_path, encoding="utf-8") as f: config = yaml.safe_load(f) if config: known_by_parser = {a.dest for a in self._actions} @@ -676,7 +676,7 @@ def load_lr_config(path: str) -> dict[str, dict[str, Any]]: """ import yaml - with open(path) as f: + with open(path, encoding="utf-8") as f: cfg = yaml.safe_load(f) if not isinstance(cfg, dict): raise ValueError(f"lr_config must be a YAML mapping, got {type(cfg).__name__}") diff --git a/modelopt/torch/puzzletron/mip/run_puzzle.py b/modelopt/torch/puzzletron/mip/run_puzzle.py index 22c8b471546..74643e2f592 100644 --- a/modelopt/torch/puzzletron/mip/run_puzzle.py +++ b/modelopt/torch/puzzletron/mip/run_puzzle.py @@ -439,7 +439,7 @@ def _get_minimal_unique_names(dicts: list[dict]) -> list[str]: def run_puzzle(args: DictConfig) -> list[str]: # Loads config from args/puzzle_profile if args.puzzle_profile is not None: - with open(args.puzzle_profile) as f: + with open(args.puzzle_profile, encoding="utf-8") as f: puzzle_profile = yaml.safe_load(f) _override_args_from_profile(args, puzzle_profile) mprint(f"Loaded Puzzle profile from {args.puzzle_profile}") diff --git a/tests/unit/onnx/conftest.py b/tests/unit/onnx/conftest.py new file mode 100644 index 00000000000..5da1ad2d663 --- /dev/null +++ b/tests/unit/onnx/conftest.py @@ -0,0 +1,43 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import contextlib + +import pytest + + +@pytest.fixture(scope="session", autouse=True) +def _prebuild_onnx_round_and_pack_ext(): + """Build the ONNX round-and-pack extension before per-test timeouts start. + + ``modelopt/onnx/quantization/extensions.py`` runs ``cppimport.imp`` at module import, and + that module is imported lazily from inside ``quant_utils.round_and_pack``. So the first test + to need it pays a full C++ compile INSIDE its own per-test timeout -- on the Windows runner + that is an MSVC build measured in minutes, and the test dies with pytest-timeout while + ``compiler.compile`` is still running. Which test pays is down to collection order, so the + failure appears to wander between runs. + + ``pyproject`` sets ``timeout_func_only``, so the per-test clock covers the call only; doing + the import here in session setup puts the build outside it. This mirrors + ``tests/gpu_megatron/conftest.py``, which prebuilds the quant CUDA extensions for the same + reason -- but it cannot reuse that helper: ``load_cpp_extension`` skips every quant extension + when CUDA is unavailable, which is exactly the case on the CPU-only Windows runner, so + ``precompile()`` would warm nothing here. + + Best-effort. The extension is an optimisation with a Python fallback -- ``extensions.py`` + already swallows its own build failures -- so a failure to prebuild must not fail the session. + """ + with contextlib.suppress(Exception): + import modelopt.onnx.quantization.extensions # noqa: F401