Skip to content

Fix squeeze value inference for scalar outputs - #2862

Open
ZedingZhang wants to merge 3 commits into
apple:mainfrom
ZedingZhang:fix/squeeze-scalar-value-inference
Open

ZedingZhang wants to merge 3 commits into
apple:mainfrom
ZedingZhang:fix/squeeze-scalar-value-inference

Conversation

@ZedingZhang

Copy link
Copy Markdown
Contributor

Summary

  • normalize scalar results from squeeze.value_inference() to the MIL input dtype
  • preserve NumPy scalar values when upstream symbolic value inference produces an object array
  • add a regression test for the concat -> split -> squeeze path from the issue

Fixes #1811.

Testing

python -m pytest \
  coremltools/converters/mil/mil/ops/tests/iOS14/test_tensor_transformation.py::TestSqueeze::test_builder_eval_rank_0 \
  coremltools/converters/mil/mil/ops/tests/iOS14/test_tensor_transformation.py::TestSqueeze::test_builder_eval_rank_0_from_split \
  coremltools/converters/mil/mil/ops/tests/iOS14/test_tensor_transformation.py::TestSqueeze::test_squeeze_value_inference_is_inplace -q

Result: 3 passed.

assert np.isclose(np.squeeze(x), v.val)

@staticmethod
def test_builder_eval_rank_0_from_split():

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The unit test should more closely follow the repo code in the GitHub issue. Specifically, it should call ct.convert.

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.

Updated the regression test to call ct.convert, as requested. This also exposed an object-dtype issue in split value inference during constant elimination, so fully known split outputs are now cast back to the input dtype.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also like the repo code, you need to call .predict on the model return by ct.convert.

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.

Updated. The regression test now calls .predict() on the model returned by ct.convert() with a concrete input and verifies that the scalar output is 1.0 on macOS 12+.

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.

Value inference errors out while creating scalars

2 participants