Skip to content

[onert/python] Support static shape modification across inference API#15184

Merged
hseok-oh merged 4 commits intoSamsung:masterfrom
ragmani:onert/python/support_static_shape_inference
Apr 21, 2025
Merged

[onert/python] Support static shape modification across inference API#15184
hseok-oh merged 4 commits intoSamsung:masterfrom
ragmani:onert/python/support_static_shape_inference

Conversation

@ragmani
Copy link
Copy Markdown
Contributor

@ragmani ragmani commented Apr 17, 2025

This commit supports static shape modification across inference API and samples.

  • common/basesession.py: add typed get_inputs_tensorinfo/get_outputs_tensorinfo helpers
  • infer/session.py:
    • normalize any -1 dims to 1 then call update_inputs_tensorinfo in __init__
    • remove obsolete compile() method
    • provide update_inputs_tensorinfo and infer with full type hints
  • samples:
    • minimal sample: build dummy inputs from tensorinfo and use infer
    • static_shape_inference example: demonstrate modifying input tensorinfo (e.g. batch size → 10) and running inference with static shapes

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

This commit supports static shape modification across inference API and samples.
- common/basesession.py: add typed get_inputs_tensorinfo/get_outputs_tensorinfo helpers
- infer/session.py:
  - normalize any `-1` dims to `1` then call `update_inputs_tensorinfo` in `__init__`
  - remove obsolete `compile()` method
  - provide `update_inputs_tensorinfo` and `run_inference` with full type hints
- samples:
  - minimal sample: build dummy inputs from tensorinfo and use `run_inference`
  - static_shape_inference example: demonstrate modifying input tensorinfo (e.g. batch size → 10) and running inference with static shapes

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 17, 2025
@ragmani ragmani requested a review from a team April 17, 2025 10:34
@ragmani
Copy link
Copy Markdown
Contributor Author

ragmani commented Apr 17, 2025

For #15172
Draft #15176

@ragmani ragmani removed the request for review from a team April 17, 2025 10:45
@ragmani ragmani added PR/NO MERGE Please don't merge. I'm still working on this :) and removed PR/ready for review It is ready to review. Please review it. labels Apr 17, 2025
@ragmani ragmani requested a review from a team April 17, 2025 11:22
@ragmani ragmani added PR/ready for review It is ready to review. Please review it. and removed PR/NO MERGE Please don't merge. I'm still working on this :) labels Apr 17, 2025
info.dims = dims # assume setter accepts a list
fixed_infos.append(info)
# update tensorinfo in session
self.update_inputs_tensorinfo(fixed_infos)
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.

(just curious)

If dynamic dimension (-1) is changed to 1 before shape interference,
I think we do not know whether dynamic shape is included in session.

In this case, how should we deal with dynamic shape?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your advice. If an input contains ‑1, it still errors, so I only applied a fragmentary fix by forcing ‑1 to 1. I just ran it with onert_run and saw the same failure, so this looks like a bug in onert. We need to fix this bug and update this code to allow ‑1 in inputs. I'll try it.

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.

(In my opinion)
It seems that the user did in a wrong scenario rather than a bug. and If the user does not know this changes(-1 -> 1), it will be difficult to debug in case of dynamic shape.
so, (optional) it would be better to print an error message that the shape should be set explicitly before runnig the unknown shape .

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There are really two modes here:

  1. Static shape modification
    Users call update_inputs_tensorinfo() once (e.g. at construction) with a fully‑specified shape (no –1s), and the session is prepared with that fixed shape.

  2. Dynamic shape modification
    If user's inputs change shape on each run, users must call update_inputs_tensorinfo(new_infos) before calling prepare(). If users skip that and any tensorinfo.dims still contains -1, users will hit the same shape error at runtime. So how should users specify dims values that change each time? If they set any of them to -1, an error will occur.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As discussed offline, I’ll keep this code unchanged and add a TODO comment.

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.

how should users specify dims values that change each time? If they set any of them to -1, an error will occur.

I agree that we can't deal with all scenarios.
but it means to maintain the error caused by unknown shape rather than changing to -1 -> 1.
and, if -1 -> 1 , it can be misunderstood that there is an unknown shape and it work without any problems, so i hope it is to solve this.

@ragmani ragmani added PR/NO MERGE Please don't merge. I'm still working on this :) and removed PR/ready for review It is ready to review. Please review it. labels Apr 18, 2025
ys44kim
ys44kim previously approved these changes Apr 18, 2025
@ragmani ragmani added PR/ready for review It is ready to review. Please review it. and removed PR/NO MERGE Please don't merge. I'm still working on this :) labels Apr 18, 2025
Copy link
Copy Markdown
Contributor

@ys44kim ys44kim left a comment

Choose a reason for hiding this comment

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

LGTM

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 👍

@hseok-oh hseok-oh merged commit 21474bc into Samsung:master Apr 21, 2025
10 checks passed
@ragmani ragmani deleted the onert/python/support_static_shape_inference branch April 21, 2025 05:10
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.

4 participants