-
Notifications
You must be signed in to change notification settings - Fork 657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support tuple inputs in extract_submodel #2267
base: main
Are you sure you want to change the base?
Conversation
@@ -170,6 +177,6 @@ def replace_inputs(func, input_vars): | |||
PASS_REGISTRY["common::dead_code_elimination"](prog) | |||
|
|||
prog.skip_all_passes = True | |||
submodel = ct.convert(prog, convert_to=backend, compute_units=model.compute_unit) | |||
submodel = ct.convert(prog, convert_to=backend, compute_units=model.compute_unit, minimum_deployment_target=func.opset_version) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly related but fixes an error I get with my example conversion script:
RuntimeWarning: You will not be able to run predict() on this Core ML model. Underlying exception message was: Error compiling model: "compiler error: Error reading protobuf spec. validator error: Description of multiarray feature 'x_cast_fp16' has FLOAT16 dataType, which is only valid in specification version >= 7. This model has version 6".
Happy to put this up as a separate PR if that's preferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a correct fix :)
@jakesabathia2 who added |
@@ -77,7 +77,14 @@ def validate_inputs(func, input_vars): | |||
reachable_vars.add(op.outputs[0]) | |||
|
|||
for op in func.operations: | |||
if all([x in reachable_vars for x in op.inputs.values()]): | |||
input_values = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @smpanaro for putting this PR!
In order to get this PR merged,
could you also add an unittest for this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem. Just added one (and also fixed one that was failing because of the opset_version change).
@jakesabathia2 Let me know if anything else is needed here! |
In the
extract_submodel
debugging util, the input reachability check doesn't support ops that take list/tuple inputs e.g. concat, stackFor example, this script:
On 8.0b1:
On this branch: