Make BundleIO ATOL/RTOL configurable per-test via runtime args#18739
Open
BryanBradfo wants to merge 1 commit intopytorch:mainfrom
Open
Make BundleIO ATOL/RTOL configurable per-test via runtime args#18739BryanBradfo wants to merge 1 commit intopytorch:mainfrom
BryanBradfo wants to merge 1 commit intopytorch:mainfrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18739
Note: Links to docs will display an error until the docs builds have been completed.
|
meta-cla
bot
added
the
CLA Signed
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes BundleIO output-comparison tolerances (ATOL/RTOL) configurable per test run for the ARM semihosting executor_runner, enabling tighter defaults while allowing per-model overrides in Cortex-M end-to-end tests.
Changes:
- Add runtime
-atol/-rtolCLI args to the semihosting runner and tighten default tolerances to1e-3. - Remove hardcoded tolerance compile definitions from the Cortex-M test runner build script.
- Add a per-model tolerance lookup table in the Cortex-M e2e script and pass tolerances through the FVP semihosting command line.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| examples/arm/executor_runner/CMakeLists.txt | Tightens default ET_ATOL/ET_RTOL CMake cache values and fixes RTOL help text. |
| examples/arm/executor_runner/arm_executor_runner.cpp | Introduces runtime -atol/-rtol overrides for BundleIO comparisons and updates defaults/usage text. |
| backends/cortex_m/test/build_test_runner.sh | Stops baking relaxed ATOL/RTOL into the runner binary at build time. |
| .ci/scripts/test_cortex_m_e2e.sh | Adds per-model tolerances and forwards them to the runner via semihosting cmd line. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
BryanBradfo
force-pushed
the
bryanbradfo/configurable-bundleio-tolerances
branch
from
1c01def to
143c585
Compare
BryanBradfo
force-pushed
the
bryanbradfo/configurable-bundleio-tolerances
branch
from
143c585 to
7abb0a6
Compare
Author
|
@pytorchbot label "release notes: none" |
pytorch-bot
bot
added
the
release notes: none
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Previously tolerances were compile-time constants (ET_ATOL=5.0, ET_RTOL=1.0) baked into the executor runner binary, applied uniformly to all models. This masked numerical regressions on precise models. Add -atol and -rtol CLI args to the semihosting runner so tolerances can be set per-model at runtime. Tighten defaults to 1e-3 and move per-model overrides to a lookup table in test_cortex_m_e2e.sh. Resolves pytorch#18424
BryanBradfo
force-pushed
the
bryanbradfo/configurable-bundleio-tolerances
branch
from
7abb0a6 to
d85f1a5
Compare
nil-is-all
added
module: arm
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.
Summary
Previously ATOL/RTOL were compile-time constants baked into the executor runner binary, applied uniformly to all models. This masked numerical regressions on models that could pass with tighter bounds.
-atoland-rtolCLI args to the semihosting runner for runtime override-DET_ATOL=5.0 -DET_RTOL=1.0from build_test_runner.shFixes #18424
Test plan
Ran
bash .ci/scripts/test_cortex_m_e2e.sh mv2on Corstone-300 FVP:Setting atol to 5.000000/Setting rtol to 2.500000Test_result: PASSwith calibrated tolerances (ATOL=5.0, RTOL=2.5)Test_result: FAILwith zero tolerances (-atol 0.0 -rtol 0.0), proving runtime override workscc @digantdesai @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell @psiddh @AdrianLundell, @rascani, @AdrianLundell