Skip to content

[BugFix][UMA] Protect target registration (#13624) - #3

Merged
tqchen merged 1 commit into
bv11from
v011cherry
Feb 20, 2023
Merged

tqchen merged 1 commit into
bv11from
v011cherry

Conversation

@tqchen

@tqchen tqchen commented Feb 20, 2023

Copy link
Copy Markdown
Owner

This PR address fixes for UMA target registration.

Changes:

  • Incorporates all proposed fixes from mentioned PR #12731
  • Address test case concerns and discussions from PR #12731
  • NEW: Already exiting target cannot be created, explicit error on this.
  • NEW: Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.

This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304
* Continues stalled PR apache#12731

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.

Signed-off-by: tqchen <tianqi.tchen@gmail.com>
@tqchen
tqchen merged commit e5906e1 into bv11 Feb 20, 2023
tqchen pushed a commit that referenced this pull request Sep 25, 2026
…e#20416)

`Hardmax` is lowered to `argmax` followed by `one_hot`, but
`relax.op.one_hot` takes its depth as a static integer attribute. When
the reduced axis has a symbolic extent, the importer passed a `tir.Var`
as the depth and failed with `TypeError: Mismatched type on argument #3
when calling relax.op.one_hot ... Expected int but got ir.Var`. For
opset 12 and below the input is first flattened to 2-D, so the reduced
extent is the product of the trailing dimensions and fails the same way
(as a `prim.Mul`) whenever any of them is symbolic, for example
`Hardmax(axis=1)` on an `[N, 3, W]` input.

This keeps the `one_hot` lowering whenever the extent is static, so the
IR for existing models is unchanged. For a symbolic extent it compares
the `keepdims` argmax index against an `arange` over the axis, broadcast
along it, and casts the result to the input dtype. `argmax` returns the
first maximum, so ties still resolve to the first index as the ONNX spec
requires.

The tests cover opsets 11 and 13 with the reduced axis symbolic, a
flattened extent that is a product involving a symbolic dim, a static
axis on an otherwise symbolic input, and inputs with ties, all compared
against onnxruntime. Six of the seven fail without this change; the
remaining one checks the unchanged static-extent path.
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