-
Notifications
You must be signed in to change notification settings - Fork 604
Fix three windows issues: extension-build timeout, 0xc000001d crash, and config I/O encoding #2461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
shengliangxu
wants to merge
23
commits into
main
Choose a base branch
from
shengliangx/prebuild-onnx-ext-off-test-clock
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
8bbe24c
Build the ONNX round-and-pack extension outside the per-test timeout
shengliangxu c8c90f1
Merge remote-tracking branch 'origin-oss/main' into shengliangx/prebu…
shengliangxu 83c6bff
Run the windows unit job in UTF-8 mode
shengliangxu c581ab9
Make text I/O encoding explicit, and keep it that way
shengliangxu 23833f8
Record the encoding change, and restore two mangled terms
shengliangxu 134b108
Cover plugins/ too, which the first encoding pass missed
shengliangxu 683d451
Format the plugins edits, and document the checker itself
shengliangxu 555cec1
Revert the explicit-encoding sweep; keep UTF-8 mode for the windows job
shengliangxu aca4a7f
Read and write YAML config as UTF-8
shengliangxu 706fbf4
Record what the windows runner is, and keep the crash dump
shengliangxu f38e29f
Merge remote-tracking branch 'origin-oss/main' into shengliangx/prebu…
shengliangxu 4e24de8
Stop windows gating merges; scope the prebuild; diagnose 0xc000001d
shengliangxu 5d065f9
Narrow 0xc000001d to a bf16 GEMM; probe the oneDNN ISA ceiling
shengliangxu 96e67a9
0xc000001d tracks the runner CPU, not the test: retarget the diagnosis
shengliangxu 774439d
Cap oneDNN's ISA on windows: 0xc000001d is a bf16 GEMM fault, not our…
shengliangxu 1fc2a00
Canary: run an nn.Linear forward, not a bare matmul
shengliangxu 5fd9fce
Fix the workflow YAML the previous commit broke; move the canary to a…
shengliangxu 1c1672b
Add the SPDX headers pre-commit requires on the two new scripts
shengliangxu b64d39c
Satisfy ruff on the two new scripts: docstrings and formatting
shengliangxu f3563c9
Drop the windows crash diagnostics; keep the fix
shengliangxu 1ad7e2d
Drop PYTHONUTF8 from the windows job; it masks the bug it looks like …
shengliangxu 5c3e3da
Merge main into shengliangx/prebuild-onnx-ext-off-test-clock
shengliangxu 6c717a7
Remove the last crash diagnostic, and changelog the encoding fix
shengliangxu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.