Skip to content

Keep the integer dtype in torch.clamp when only one bound is given - #2839

Open
LeSingh1 wants to merge 1 commit into
apple:mainfrom
LeSingh1:fix/clamp-int-single-bound
Open

Keep the integer dtype in torch.clamp when only one bound is given#2839
LeSingh1 wants to merge 1 commit into
apple:mainfrom
LeSingh1:fix/clamp-int-single-bound

Conversation

@LeSingh1

Copy link
Copy Markdown
Contributor

torch.clamp(x, min=...) and torch.clamp(x, max=...) on an int tensor come out of the converter as fp32, and the values are rounded to what fp32 can hold.

When only one bound is given, the converter substitutes np.finfo(np.float32).min / .max for the missing one. That sentinel is a float const, so promote_input_dtypes promotes x along with it and the whole op becomes fp32. PyTorch keeps the integer dtype unless one of the given bounds is a float.

Repro on main:

import torch, numpy as np, coremltools as ct

class M(torch.nn.Module):
    def forward(self, x): return torch.clamp(x, min=0)

x = torch.tensor([16777217, 16777219, 2000000001, -16777217, 123456789, 0, -1, 5],
                 dtype=torch.int32)
m = M().eval()
ts = torch.jit.trace(m, x)
ml = ct.convert(ts, inputs=[ct.TensorType(name="x", shape=x.shape, dtype=np.int32)],
                convert_to="mlprogram", minimum_deployment_target=ct.target.iOS17,
                compute_precision=ct.precision.FLOAT32, compute_units=ct.ComputeUnit.CPU_ONLY)
print(ml._mil_program)
print("torch :", m(x).numpy())
print("coreml:", list(ml.predict({"x": x.numpy()}).values())[0])

Before:

main[CoreML7](%x: (8,int32)(Tensor)) {
  block5() {
    %x_promoted: (8,fp32)(Tensor) = cast(x=%x, dtype="fp32", name="cast_0")
    %clip_0: (8,fp32)(Tensor) = clip(x=%x_promoted, alpha=0.0, beta=3.4028234663852886e+38, name="clip_0")
  } -> (%clip_0)
}

torch : [  16777217   16777219 2000000001          0  123456789          0          0          5]
coreml: [1.6777216e+07 1.6777220e+07 2.0000000e+09 0.0000000e+00 1.2345679e+08 0.0000000e+00 0.0000000e+00 5.0000000e+00]

Four of the eight values are wrong, not just the dtype: 16777217 -> 16777216, 16777219 -> 16777220, 2000000001 -> 2000000000, 123456789 -> 123456792. Anything downstream that expects an integer (indices, for instance) gets a float instead.

After:

main[CoreML7](%x: (8,int32)(Tensor)) {
  block5() {
    %minimum_0: (8,int32)(Tensor) = minimum(x=%x, y=2147483647, name="minimum_0")
    %maximum_0: (8,int32)(Tensor) = maximum(x=%minimum_0, y=0, name="maximum_0")
  } -> (%maximum_0)
}

torch : [  16777217   16777219 2000000001          0  123456789          0          0          5]
coreml: [  16777217   16777219 2000000001          0  123456789          0          0          5]

The fix takes the sentinel from x's own dtype range instead of always from fp32. A float bound that is actually passed still promotes as before, so torch.clamp(int_x, min=2.5) remains fp32, matching PyTorch.

Testing

New TestElementWiseUnary::test_clamp_int_input_single_bound (min-only and max-only). 4 of its 8 cases fail on main and all 8 pass here. The rest of TestElementWiseUnary, including the existing test_clamp, test_clamp_int_input, test_clamp_non_const_range and test_clamp_min_max, still passes.

@TobyRoseman

Copy link
Copy Markdown
Collaborator

This change looks good.

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

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.

2 participants