add_int16_cast adds uint16 cast causing underflow of int16-compatible int32 value

[kasper0406]

The current add_int16_cast implementation has a side-effect in the should_cast_parameter method. It updates the target_dtype member when it is being called on different parameters. This target_dtype value is then used in the transform_op method, to add casts. The issue is that, in certain cases, the target_dtype does not get correctly reset between parameters, and unintended uint16 casts can occur!

The added test case will fail with:

E            ACTUAL: array([    0,     0,     0,     4, 65524,     4, 65515], dtype=int32)
E            DESIRED: array([  0,   0,   0,   4, -12,   4, -21], dtype=int32)

An easy fix would have been to reset the target_dtype state in every call to should_cast_parameter.
However, I believe this design is quite confusing, and likely to cause further issues, so I've gone ahead and refactored the code to make should_cast_parameter return an optional with the parameter's target dtype instead of having a side effect.

The transform_function_signatures method iterates the function inputs, and uses self.target_dtype to modify the inputs. This function did not use should_cast_parameter before as well, so depending on if it is called before/after the parameter conversion code, it may use a different self.target_dtype based on parameter ordering. In the new version, it will always use the default target_dtype. I am not sure what the correct behavior is here.

[TobyRoseman]

Thanks for the pull request. The fix looks good to me.

CI: https://gitlab.com/coremltools1/coremltools/-/pipelines/2189637393