Skip to content

Cortex-M backend: Make BundleIO ATOL/RTOL configurable per-test via runtime args #18739

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.

Sign up for GitHub

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
BryanBradfo wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Cortex-M backend: Make BundleIO ATOL/RTOL configurable per-test via runtime args #18739

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .ci/scripts/test_cortex_m_e2e.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@
set -eux

MODEL=$1

# Per-model BundleIO tolerances. Int8 quantized models need relaxed bounds
# due to quantization error accumulating through layers. Models not listed
# use the tight default (1e-3); override here for models that need it.
declare -A MODEL_ATOL=( [mv2]="5.0" [mv3]="5.0" )
declare -A MODEL_RTOL=( [mv2]="2.5" [mv3]="2.5" )
ATOL="${MODEL_ATOL[$MODEL]:-1e-3}"
RTOL="${MODEL_RTOL[$MODEL]:-1e-3}"
Comment thread
BryanBradfo marked this conversation as resolved.

mkdir -p "./cortex_m_e2e/${MODEL}"
WORK_DIR=$(realpath "./cortex_m_e2e/${MODEL}")

Expand Down Expand Up @@ -51,7 +60,7 @@ FVP_Corstone_SSE-300_Ethos-U55 \
-C cpu0.semihosting-heap_limit=0 \
-C "cpu0.semihosting-cwd=${WORK_DIR}" \
-C "ethosu.extra_args='--fast'" \
-C "cpu0.semihosting-cmd_line='executor_runner -m ${MODEL}.bpte -i dummy.bin -o out'" \
-C "cpu0.semihosting-cmd_line='executor_runner -m ${MODEL}.bpte -i dummy.bin -o out -atol ${ATOL} -rtol ${RTOL}'" \
Copy link
Copy Markdown
Contributor

@digantdesai digantdesai Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why does the error bound enforcing logic has to live in the runner? Can the runner report the ATOL, RTOL, and other stats by comparing against the bundled output and then we can compare it outside?

Rationale is, (1) it will save cycles, and error reporting from the runtime on Cortex-M, (2) we can have it more flexible if we validate it outside. And support multiple PTEs for a same runner - through semihosting or something else.

These two doesn't have to be mutually exclusive, i.e. we support both, (1) reporting detailed comparison stats for outside comparison, (2) optionally allowing users to bake in thresholds.

Copy link
Copy Markdown
Collaborator

@AdrianLundell AdrianLundell Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add to this we are already running a number of e2e tests on FVP in https://github.com/pytorch/executorch/tree/main/backends/cortex_m/test/models with individual tolerances, isn't that good enough to catch regressions?

-a "${ELF}" \
--timelimit 300 2>&1 | tee "${LOG_FILE}" || true

Expand Down
2 changes: 1 addition & 1 deletion backends/cortex_m/test/build_test_runner.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ aten::unsqueeze_copy.out,\
aten::select_copy.int_out,\
aten::amax.out"

${build_executor_runner} --pte=semihosting --bundleio --target=ethos-u55-128 --output="${build_root_test_dir}" --select_ops_list="${select_ops_list}" --extra_build_flags="-DET_ATOL=5.0 -DET_RTOL=1.0"
${build_executor_runner} --pte=semihosting --bundleio --target=ethos-u55-128 --output="${build_root_test_dir}" --select_ops_list="${select_ops_list}"
20 changes: 9 additions & 11 deletions examples/arm/executor_runner/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ option(ET_LOG_DUMP_OUTPUT "Dump output in log" ON)

option(ET_BUNDLE_IO "Set to compile in BundleIO support" OFF)
set(ET_ATOL
"0.01"
"1e-3"
CACHE STRING "Set atol to use for BundleIO testing (Requires ET_BUNDLE_IO)"
)
set(ET_RTOL
"0.01"
CACHE STRING "Set atol to use for BundleIO testing (Requires ET_BUNDLE_IO)"
"1e-3"
CACHE STRING "Set rtol to use for BundleIO testing (Requires ET_BUNDLE_IO)"
)
Comment thread
BryanBradfo marked this conversation as resolved.

option(
Expand Down Expand Up @@ -415,14 +415,12 @@ endif()

if(ET_BUNDLE_IO)
target_compile_definitions(arm_executor_runner PUBLIC -DET_BUNDLE_IO)
endif()

if(ET_ATOL)
target_compile_definitions(arm_executor_runner PUBLIC ET_ATOL=${ET_ATOL})
endif()

if(ET_RTOL)
target_compile_definitions(arm_executor_runner PUBLIC ET_RTOL=${ET_RTOL})
if(DEFINED ET_ATOL)
target_compile_definitions(arm_executor_runner PUBLIC ET_ATOL=${ET_ATOL})
endif()
if(DEFINED ET_RTOL)
Comment thread
BryanBradfo marked this conversation as resolved.
target_compile_definitions(arm_executor_runner PUBLIC ET_RTOL=${ET_RTOL})
endif()
endif()

# Devtools ETDump: Speed and dumping output
Expand Down
42 changes: 32 additions & 10 deletions examples/arm/executor_runner/arm_executor_runner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@
* use bpte with bundled input and output refdata to
* compare output.
* See also ET_ATOL and ET_RTOL
* ET_ATOL - The atol used to compare the output and ref data
* when using ET_BUNDLE_IO ET_RTOL - The rtol used to compare the
* output and ref data when using ET_BUNDLE_IO
* ET_ATOL - The absolute tolerance used to compare output and
Copy link
Copy Markdown
Contributor

@psiddh psiddh Apr 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Thinking aloud) If any test runner other than test_cortex_m_e2e.sh invokes this, without passing -atol/-rtol at runtime, it will silently use the new tight 1e-3 default instead of the old 5.0/1.0. We should keep an eye on it . hopefully there are no errors as a result of this change.

[EDIT:]

  1. .github/workflows/test-mcu-cortex-m.yml (line 44) — builds the binary, then runs pytest on backends/cortex_m/test/. Those pytest tests go through backends/arm/test/runner_utils.py → run_corstone()
    which does NOT pass -atol/-rtol flags. So these tests would hit the new tight 1e-3 default.
  2. .github/workflows/trunk.yml (line 1087) — builds the binary, then calls test_cortex_m_e2e.sh for mv2/mv3. This script does pass -atol/-rtol per the PR. This caller is fine.

Copy link
Copy Markdown
Author

@BryanBradfo BryanBradfo Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the pytest path uses run_corstone() with plain .pte files, so BundleIO verification never runs there. BundleIO is also used by test_arm_baremetal.sh (via run.sh --bundleio), which doesn't pass -atol/-rtol and uses the compile-time default. CI confirms those tests pass with the tighter 1e-3 default.

* ref data when using ET_BUNDLE_IO.
* Can be overridden at runtime with -atol <value>.
* ET_RTOL - The relative tolerance used to compare output and
* ref data when using ET_BUNDLE_IO.
* Can be overridden at runtime with -rtol <value>.
*
* Devtools ETDump: Speed and dumping output
*
Expand Down Expand Up @@ -221,15 +224,15 @@ unsigned char __attribute__((
const size_t testset_idx = 0; // BundleIO test indexes to test if used

#if defined(ET_ATOL)
const float et_atol = ET_ATOL;
float et_atol = ET_ATOL;
#else
const float et_atol = 0.01;
float et_atol = 1e-3f;
#endif

#if defined(ET_RTOL)
const float et_rtol = ET_RTOL;
float et_rtol = ET_RTOL;
#else
const float et_rtol = 0.01;
float et_rtol = 1e-3f;
#endif

#endif
Expand Down Expand Up @@ -1192,13 +1195,16 @@ int main(int argc, const char* argv[]) {
ET_LOG(Fatal, "Not right number of parameters!");
ET_LOG(
Fatal,
#if defined(ET_BUNDLE_IO)
"app -m model.pte -i input.bin [-i input2.bin] -o output_basename [-atol 0.001] [-rtol 0.001]");
Comment thread
BryanBradfo marked this conversation as resolved.
#else
"app -m model.pte -i input.bin [-i input2.bin] -o output_basename");
#endif
Comment thread
BryanBradfo marked this conversation as resolved.
ET_LOG(Fatal, "Exiting!");
_exit(1);
}
ET_LOG(Info, " %s", argv[0]);
for (int i = 1; i < argc; i++) {
ET_LOG(Info, " %s %s", argv[i], argv[++i]);
for (int i = 0; i < argc; i++) {
ET_LOG(Info, " %s", argv[i]);
}
#else
(void)argc;
Expand Down Expand Up @@ -1265,6 +1271,22 @@ int main(int argc, const char* argv[]) {
} else if (std::strcmp(argv[i], "-o") == 0) {
// store the base filename to write output to.
ctx.output_basename = argv[++i];
#if defined(ET_BUNDLE_IO)
} else if (std::strcmp(argv[i], "-atol") == 0) {
if (i + 1 < argc && sscanf(argv[++i], "%f", &et_atol) == 1) {
ET_LOG(Info, "Setting atol to %f", et_atol);
} else {
ET_LOG(Fatal, "Invalid or missing value for -atol");
_exit(1);
}
} else if (std::strcmp(argv[i], "-rtol") == 0) {
if (i + 1 < argc && sscanf(argv[++i], "%f", &et_rtol) == 1) {
ET_LOG(Info, "Setting rtol to %f", et_rtol);
} else {
ET_LOG(Fatal, "Invalid or missing value for -rtol");
_exit(1);
}
#endif
}
}
#endif
Expand Down
Loading