Skip to content

[onert/python] Support dynamic shapes(on-the-fly)#15205

Merged
hseok-oh merged 3 commits intoSamsung:masterfrom
ragmani:onert/python/support_dynamic_shape
Apr 30, 2025
Merged

[onert/python] Support dynamic shapes(on-the-fly)#15205
hseok-oh merged 3 commits intoSamsung:masterfrom
ragmani:onert/python/support_dynamic_shape

Conversation

@ragmani
Copy link
Copy Markdown
Contributor

@ragmani ragmani commented Apr 22, 2025

This commit support dynamic shapes and add dynamic_shape_inference sample.

  • Remove the old “replace -1 with 1” placeholder in the constructor
  • In infer(), on first call:
    • Inspect each tensorinfo.dims and for any -1 replace it with the matching inputs_array[i].shape[j]
    • Validate that any non--1 dims match the actual input shape, raising on mismatch
    • Call update_inputs_tensorinfo(), then prepare() and set_outputs() once
  • Bundle the above into the “auto‑dynamic” path instead of hard‑coding 1’s
  • Add dynamic_shape_inference.py sample to show 10 runs with random shapes
  • Update minimal.py to retain its original static‑shape example logic

ONE-DCO-1.0-Signed-off-by: ragmani ragmani0216@gmail.com

@ragmani ragmani added the PR/ready for review It is ready to review. Please review it. label Apr 22, 2025
@ragmani ragmani requested a review from a team April 22, 2025 11:08
@ragmani ragmani force-pushed the onert/python/support_dynamic_shape branch from 415ff1b to d85affe Compare April 22, 2025 11:13
@ragmani
Copy link
Copy Markdown
Contributor Author

ragmani commented Apr 22, 2025

For #15172

@ragmani
Copy link
Copy Markdown
Contributor Author

ragmani commented Apr 22, 2025

This PR follows #15171 (comment)

@hseok-oh
Copy link
Copy Markdown
Contributor

hseok-oh commented Apr 24, 2025

I have two question

  • Is update_inputs_tensorinfo() user API or internal function?
    • If that is user API, do we need to support user API to set tensor info? Can't we support static shape inference internally by using infer's numpy input tensor list's shape information when shape is not changed on each inference?
  • Why python sample code file dynamic_shape_inference.py is in src/ directory? Does python file need src/ directory?

Comment thread runtime/onert/api/python/package/infer/session.py Outdated
@ragmani
Copy link
Copy Markdown
Contributor Author

ragmani commented Apr 24, 2025

  • Is update_inputs_tensorinfo() user API or internal function?
    • If that is user API, do we need to support user API to set tensor info? Can't we support static shape inference internally by using infer's numpy input tensor list's shape information when shape is not changed on each inference?

Framework Comparison: Input-Tensor Shape Handling

How Keras (TensorFlow), PyTorch and ONERT manage input shapes—static, dynamic (on-the-fly) and unmodified—in their Python APIs.

Framework Graph Model Shape Handling Python API Typical Workflow Notes
Keras (TF 2.x) Eager + optional tf.function (static) Static: locked at tf.function trace
Dynamic: only in pure eager mode; mixing shapes in one trace fails
TF retracing docs
model = tf.keras.Model(...)
tf.function(model)
1. Define model
2. (Optionally) wrap in @tf.function
3. Call model(x)
4. To change non-batch dims, re-trace/re-build model
Retracing on shape change degrades performance; only batch dimension (None) is variable in Input(shape=(None,…)).
PyTorch Define-by-run dynamic graph Dynamic: any shape each forward call
Static: via TorchScript tracing for fixed-shape optimization
TorchScript docs
output = model(input_tensor) 1. Define nn.Module
2. Call model(x) with any shape
3. (Optional) torch.jit.trace for static-shape performance
No separate compile step—shape changes incur no overhead in eager mode; TorchScript can freeze shapes.
ONERT C-API session with Python wrapper Static override: update_inputs_tensorinfo() + infer()
Dynamic: infer(inputs_array) alone
Unmodified: infer() w/o changes
ONERT API
sess = infer.session(...)
sess.infer(inputs)
Static: get tensorinfo → modify dims → update_inputs_tensorinfo()infer()
Dynamic: call infer(inputs_array)
Unmodified: infer() only
Dynamic mode may use more memory/be slower

Policy Consideration

Deciding whether to allow inputs that differ from the original model tensorinfo is a policy issue:

  • Internal-only update_inputs_tensorinfo()

    • Pros: Simplifies API surface; hides complexity from users.
    • Cons: Harder to catch and report user mistakes in input shapes; unintended fall-through into dynamic mode may increase memory usage and slow execution.
  • Exposed user API update_inputs_tensorinfo()

    • Pros: Maximum flexibility—users can explicitly override shapes.
    • Cons: Must implement robust error handling for incorrect shapes; risk of performance overhead if users inadvertently trigger dynamic mode.

Trade-off: internal-only hides complexity but limits flexibility; user-exposed offers control at the cost of extra error-handling and potential unintended dynamic-mode overhead.

@ragmani
Copy link
Copy Markdown
Contributor Author

ragmani commented Apr 24, 2025

  • Why python sample code file dynamic_shape_inference.py is in src/ directory? Does python file need src/ directory?

I agree that dynamic_shape_inference.py doesn’t belong under src/. Could you suggest a more appropriate directory for this example?

ragmani added 2 commits April 24, 2025 05:57
This commit support dynamic shapes and add dynamic_shape_inference sample.
- Remove the old “replace -1 with 1” placeholder in the constructor
- In `infer()`, on first call:
  - Inspect each `tensorinfo.dims` and for any `-1` replace it with the matching `inputs_array[i].shape[j]`
  - Validate that any non-`-1` dims match the actual input shape, raising on mismatch
  - Call `update_inputs_tensorinfo()`, then `prepare()` and `set_outputs()` once
- Bundle the above into the “auto‑dynamic” path instead of hard‑coding 1’s
- Add `dynamic_shape_inference.py` sample to show 10 runs with random shapes

ONE-DCO-1.0-Signed-off-by: ragmani <ragmani0216@gmail.com>
ONE-DCO-1.0-Signed-off-by: ragmani <ragmani0216@gmail.com>
@ragmani ragmani force-pushed the onert/python/support_dynamic_shape branch from f46a565 to a696f38 Compare April 24, 2025 06:00
@hseok-oh
Copy link
Copy Markdown
Contributor

hseok-oh commented Apr 24, 2025

unintended fall-through into dynamic mode may increase memory usage and slow execution.

I don't think user can enable unintended dynamic mode. To enable dynamic mode, user must change input shape after 1st inference. If user set input shape before 1st inference and doesn't change after, then runtime will reuse static shape inference mode's result only.

Harder to catch and report user mistakes in input shapes

Agree. It is trade-off.

IMO, it looks Pros > Cons when API automatically set input shape. Could you share your opinion?

@hseok-oh
Copy link
Copy Markdown
Contributor

I agree that dynamic_shape_inference.py doesn’t belong under src/. Could you suggest a more appropriate directory for this example?

It's independent issue with this PR :)
I think we don't need src directory: runtime/onert/sample/minimal-python/static_shape_inference.py. Then we need to move all files in runtime/onert/sample/minimal-python/src into runtime/onert/sample/minimal-python.

@ragmani
Copy link
Copy Markdown
Contributor Author

ragmani commented Apr 24, 2025

I don't think user can enable unintended dynamic mode. To enable dynamic mode, user must change input shape after 1st inference. If user set input shape before 1st inference and doesn't change after, then runtime will reuse static shape inference mode's result only.

You're right. The dynamic mode can be enabled only when input shapes change for each infer() even if we hide update_inputs_tensorinfo internally.

IMO, it looks Pros > Cons when API automatically set input shape. Could you share your opinion?

How about providing a warning message instead of throwing an exception?

import warnings

def infer(input_shape=None, data=None):
    ...
        warnings.warn(
            f"infer() called with input_shape={input_shape}, which differs from model’s expected shape={expected}. "
            "Ensure this is intended.",
            UserWarning
   ...

@ragmani
Copy link
Copy Markdown
Contributor Author

ragmani commented Apr 24, 2025

I think we don't need src directory: runtime/onert/sample/minimal-python/static_shape_inference.py. Then we need to move all files in runtime/onert/sample/minimal-python/src into runtime/onert/sample/minimal-python.

OK, I will move them in another PR :)

@ragmani
Copy link
Copy Markdown
Contributor Author

ragmani commented Apr 29, 2025

@hseok-oh @zetwhite PTAL

@zetwhite
Copy link
Copy Markdown
Contributor

@zetwhite PTAL

Ah, I thought you're going to add a warning message like this - #15205 (comment).
That's why I didn't review this PR earlier.

zetwhite
zetwhite previously approved these changes Apr 29, 2025
Copy link
Copy Markdown
Contributor

@zetwhite zetwhite left a comment

Choose a reason for hiding this comment

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

LGTM 👍 👍

@zetwhite zetwhite requested a review from a team April 29, 2025 02:21
Comment thread runtime/onert/api/python/package/infer/session.py Outdated
@ragmani
Copy link
Copy Markdown
Contributor Author

ragmani commented Apr 29, 2025

@zetwhite

Ah, I thought you're going to add a warning message like this - #15205 (comment).
That's why I didn't review this PR earlier.

Ah, you are right, I was waiting for it. Sorry for bother you.

@hseok-oh
Copy link
Copy Markdown
Contributor

How about providing a warning message instead of throwing an exception?

👍 <- Looks good

Copy link
Copy Markdown
Contributor

@hseok-oh hseok-oh left a comment

Choose a reason for hiding this comment

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

LGTM

@zetwhite
Copy link
Copy Markdown
Contributor

Ah, you are right, I was waiting for it. Sorry for bother you.

No problem at all! Thanks for your work :)

@hseok-oh hseok-oh merged commit af6bd7a into Samsung:master Apr 30, 2025
10 checks passed
@ragmani ragmani deleted the onert/python/support_dynamic_shape branch April 30, 2025 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR/ready for review It is ready to review. Please review it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants