pnnx: handle float16 scalar constant when importing onnx - #6852
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 143e074fc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # into primitive ops whose 0.5 / sqrt(2) coefficients become float16 scalar | ||
| # constants, which is what exercises the float16 scalar-constant path in the | ||
| # onnx importer | ||
| torch.onnx.export(net, (x,), "test_F_gelu_fp16.onnx", opset_version=18, dynamo=True) |
There was a problem hiding this comment.
Gate dynamo exporter test on torch versions
This test is now registered for every pnnx CTest run, including matrix jobs such as Python 3.9 with torch 1.11.0 in .github/workflows/pnnx.yml before the workflow runs ctest. In those older torch environments torch.onnx.export does not accept the dynamo exporter option, so the test fails with a TypeError before it ever reaches the importer; please skip/gate this case on a torch version that supports dynamo=True.
Useful? React with 👍 / 👎.
| torch.onnx.export(net, (x,), "test_F_gelu_fp16.onnx", opset_version=18, dynamo=True) | ||
|
|
||
| # ort inference | ||
| sess = ort.InferenceSession("test_F_gelu_fp16.onnx") |
There was a problem hiding this comment.
Avoid running fp16 GELU through ORT CPU
In the pnnx workflow the Python side installs the CPU onnxruntime package, so this session runs on CPU; the exported fp16 GELU decomposition uses fp16 primitive ops, but ONNX Runtime's CPU kernel table lists Erf for CPU as tensor(float) only (operator kernels). On torch versions new enough to export the model, this makes the reference inference fail in ORT instead of testing pnnx; use the PyTorch output as the reference, cast the ORT graph to fp32, or skip unless a float16-capable EP is available.
Useful? React with 👍 / 👎.
Signed-off-by: 林晨 (Leo Cheng) <leo-cheng@vip.qq.com>
143e074 to
d0d4cda
Compare
|
Both fixed: the test now gates on |
The onnx importer's scalar-constant loader in
pass_onnx.cpphandles INT32/INT64/FLOAT/BOOL but has no FLOAT16 branch, so a float16 scalar constant leavesparams["value"]unset. Laterpass_level3/eliminate_noop_math.cppcallsparams.at("value")and throwsstd::out_of_range(map::at), aborting the conversion.This adds a FLOAT16 branch that reuses the existing
float16_to_float32helper, reading eitherraw_dataor theint32_databit pattern, so float16 scalar constants are handled like the other scalar types.It reproduces with a float16 GELU exported at opset 18 (dynamo), which lowers gelu into primitives whose
0.5/sqrt(2)coefficients become float16 scalar constants. Addedtests/onnx/test_F_gelu_fp16.pyfor it - the test aborts before this change and passes after.Closes #6817