Skip to content

Add transformers tool test to pipeline - #7959

Merged
gh-yewang merged 31 commits into
masterfrom
wangye/pipeline
Jun 9, 2021
Merged

Add transformers tool test to pipeline#7959
gh-yewang merged 31 commits into
masterfrom
wangye/pipeline

Conversation

@gh-yewang

@gh-yewang gh-yewang commented Jun 5, 2021

Copy link
Copy Markdown
Contributor

Description: Describe your changes.

  1. Setup a pipeline for transformers tool test in Linux CPU machine(an extra <1000s used on Linux CPU pipeline)
  2. Add --enable_transformers_tool_test in build.py so that we could only enable it in target machines
  3. pytorch1.4 related tests are removed
  4. add back some onnx files
  5. failed tests are temporarily disabled. Plan to investigate and resolve in future PRs.
  6. torch/transformers..package version is hardcoded to avoid unexpected pipeline breaking

Motivation and Context

  • Why is this change required? What problem does it solve?
  • If it fixes an open issue, please link to the issue here.

@gh-yewang
gh-yewang requested a review from tianleiwu June 7, 2021 21:19
@gh-yewang gh-yewang changed the title [WIP]Add transformers tool test to pipeline Add transformers tool test to pipeline Jun 7, 2021
@gh-yewang
gh-yewang marked this pull request as ready for review June 8, 2021 06:00
@gh-yewang
gh-yewang requested a review from a team as a code owner June 8, 2021 06:00
Comment thread onnxruntime/test/python/transformers/test_attention_fusion.py Outdated
Comment thread onnxruntime/test/python/transformers/test_shape_infer_helper.py
Comment thread onnxruntime/test/python/transformers/test_optimizer.py Outdated
Comment thread onnxruntime/test/python/transformers/test_shape_infer_helper.py
@gh-yewang
gh-yewang requested a review from tianleiwu June 8, 2021 20:43
Comment thread tools/ci_build/build.py
if args.enable_transformers_tool_test:
required = {
'numpy==1.19.2', 'coloredlogs==15.0', 'tf2onnx==1.8.5', 'transformers==4.6.1',
'torch==1.8.1', 'tensorflow==2.5.0', 'onnxconverter-common==1.8.1', 'psutil'}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tf2onnx and tensorflow might not be required since slow tests will not run in pipeline.

@gh-yewang
gh-yewang merged commit d433aa2 into master Jun 9, 2021
@gh-yewang
gh-yewang deleted the wangye/pipeline branch June 9, 2021 02:44
titaiwangms added a commit that referenced this pull request Jul 6, 2026
…master #7959 node-data deletion)

Agent-signed-off: Architect (dae3a478) [claude-opus-4.8 via copilot]
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
titaiwangms added a commit that referenced this pull request Jul 16, 2026
…9) + retiring equivalence proof + corpus-collapse tripwire (#29504)

### Motivation
ONNX onnx/onnx#7959 removes the on-disk `onnx/backend/test/data/node/`
corpus (targeted for ONNX 1.23), replacing it with on-the-fly in-memory
generation. ORT's C++ `onnx_test_runner` reads that corpus from disk —
after the deletion it would load **0 node tests and silently exit 0**
(green CI on a corpus that no longer exists). This PR detaches ORT from
the deleted artifacts *ahead* of the bump.

### What this does (3 pieces)
1. **Detach** — a build-time materializer
(`tools/python/materialize_onnx_node_tests.py`) regenerates the node
corpus to disk from ONNX's surviving generator (`collect_testcases`), so
the C++ runner keeps reading from disk unchanged. EP-agnostic
(CPU/CUDA/QNN share one materialized tree).
2. **Equivalence proof** — a *retiring* test
(`onnxruntime/test/python/onnx_node_test_equivalence_test.py` +
`tools/python/compare_node_test_corpora.py`) proves the materialized
corpus is byte-identical to the original (modulo a documented ULP band);
it auto-skips once the ONNX on-disk oracle disappears post-#7959.
3. **Cause-agnostic tripwire** — build-time `--min-cases` gate (FATALs
the build on every MATERIALIZE=ON leg if generation < floor) + a runtime
`-m` floor on the CPU node ctest, turning silent-green-on-empty into a
loud red.

### Key decisions
- **numpy**: cmake configure = HARD FATAL on off-pin numpy (build-env
reproducibility across CI legs); Python materializer = SOFT WARN
(standalone advisory). Different layers, different questions. The onnx
pin stays HARD on both sides.
- **QNN legs**: repointed to the materialized tree + pinned onnx/numpy
installed pre-build. Node-dir runs carry a **low `-m 1` collapse
sentinel** (NOT the 1500 build floor): `-e qnn` legitimately reduces the
collected set to ~1529, so a 1500 runtime floor would be a false-red
timebomb on the next opset bump — the strict count is enforced at build
time via `--min-cases`, while `-m 1` (zero false-red risk) catches a
per-leg `MATERIALIZE=OFF` that still runs the node dir. The android
leg's single-case `cp -r` source is repointed to the materialized tree
(the old `data/node` source vanishes post-#7959).

### Testing
- Runtime tripwire empirically verified: empty/truncated corpus → runner
FATAL (nonzero); full → exit 0; default (no `-m`) unchanged.
- Equivalence: 1799-case byte-probe (dir-set match; byte-identical
modulo the documented ULP band).
- cmake configure verified to generate cleanly on latest main
(ep_context + node-test regions coexist, no collisions).
- **[needs-run]** on a pinned-numpy CI box: the `MATERIALIZE=ON` inner
path (actual materialization + the two add_test bodies) — this dev box
is off-pin so the numpy gate FATALs by design.

Relates to: onnx/onnx#7959


---
## Cross-consumer viability note (re: onnx/onnx#7959)

While detaching, we assessed all of ORT's node-test consumers. The
in-memory generator approach behind #7959 works cleanly for
**single-version** consumers (C++, C#, docs), but has a gap for
consumers needing a **historical multi-opset matrix**:
- ORT's JS/web tests pull node data for opset 7–21 from 15 immutable
`rel-*` release archives. Since onnx's generator is single-version, old
opsets can't be regenerated from a new onnx.
- Those consumers stay green post-#7959 only because released branches
are immutable — there is no generator-based path to add a *new*
post-#7959 opset for them.

Net: workable for single-version consumers; the multi-version case would
need a small per-version serialization utility or a documented migration
note upstream.
---

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
titaiwangms added a commit that referenced this pull request Jul 16, 2026
…reen if onnx pin bumps past #7959

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
titaiwangms added a commit that referenced this pull request Jul 16, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
titaiwangms added a commit that referenced this pull request Jul 27, 2026
…track QNN materialize deps (#29577)

Follow-up to the merged #29504 (ONNX node-test detachment). This PR
consolidates **all** post-#29504 follow-up work into a single change
set. Changes C/D/E address @tianleiwu's non-blocking post-merge review
comments; A/B were the original scope of this PR. None change the
behavior of the shipped feature — this is hardening + hygiene + doc
detachment.

### (A) C# — loud-fail guard on the macOS node-test symlink
`csharp/test/Microsoft.ML.OnnxRuntime.EndToEndTests/runtest.sh`
symlinked the ONNX on-disk node-test corpus. Once the onnx pin advances
past onnx/onnx#7959 (which deletes that corpus) the symlink target
vanishes and the suite would silently pass with zero cases. Added a
loud-fail guard: enforce a discovered-case floor (1000) and keep the
immutable v1.13.1 pin, so corpus absence becomes a red instead of a
silent-green.

### (B) JS — post-#7959 opset caveat in `prepare-onnx-node-tests.ts`
Documented that once onnx ships without the on-disk node-test data
(onnx/onnx#7959), post-#7959 opsets cannot use the `rel-*` archive
download path — a caveat for the next opset bump.

### (C = N1) cmake — drop no-op ctest `DEPENDS` + document real ordering
`cmake/onnxruntime_unittests.cmake`: the two `set_tests_properties(...
PROPERTIES DEPENDS onnx_node_tests_materialized)` calls named an
`add_custom_target(... ALL)`, not a registered test, so a CTest
test-level `DEPENDS` on it is a silent no-op. Removed both and
documented the real ordering guarantee: the `ALL` target materializes
the corpus during the build (before ctest), and the `-m` / `--min-cases`
tripwire fails loud on a missing/partial corpus.

### (D = N2) conf.py — hermetic in-memory doc model
`docs/python/conf.py` urlretrieved `.../node/test_sigmoid/model.onnx`
from GitHub at doc-build time — a latent 404 once onnx/onnx#7959 deletes
that path. Replaced with an in-memory `onnx.helper` single-node Sigmoid
graph (input `x` → output `y`, float32 `[3,4,5]`) written to the same
destination. Keeps the doc build hermetic and drop-in compatible.

### (E = N3-5) requirements file for QNN/Android materialize deps +
SKILL.md
The four QNN/Android CI legs used inline `pip install onnx==...
"numpy==...; ..."` pins (invisible to Dependabot) to feed the
`onnxruntime_MATERIALIZE_ONNX_NODE_TESTS` gate. Added a dedicated
minimal `requirements-materialize-onnx-node-tests.txt` (onnx + numpy
only, with hybrid numpy env markers `2.2.6` for `python_version<'3.11'`,
`2.4.2` for `>='3.11'`) and repointed all four legs to `pip install -r`
(preserving `--user` on linux + android; the two Windows legs omit it).
Placed at the repo root because `dependabot.yml` pins pip to `directory:
"/"` (non-recursive). Pins stay locked in lockstep with the CMake gate,
which HARD-FATALs on `installed != pinned`. Updated the
`onnx-opset-bump-checklist` SKILL.md gotcha-p to reflect the new file
(reversing its prior "inline by design" note) and added it to the numpy
pin sync-site list.

---
Consolidates the standalone #29746 (now closed) per author request.
Triple-review passed on both change sets independently. Merge after
#29504 (already merged), which this references.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants