Skip to content

Fix log2 and log10 converters to handle integer input#2735

Open
joaopedroassad wants to merge 1 commit into
apple:mainfrom
joaopedroassad:fix-log2-log10-integer-input
Open

Fix log2 and log10 converters to handle integer input#2735
joaopedroassad wants to merge 1 commit into
apple:mainfrom
joaopedroassad:fix-log2-log10-integer-input

Conversation

@joaopedroassad

Copy link
Copy Markdown

Summary

The PyTorch log2 and log10 converters pass their input straight into MIL's mb.log, which only accepts floats (fp16/fp32). The issue is that torch.log2 and torch.log10 take integer tensors and return float, so converting a model that calls either of them on an int tensor currently fails MIL type inference.

The neighboring log and log1p converters already handle this by casting int input to float first (that cast was added to log in #2017). This change gives log2 and log10 the same two-line guard:

if types.is_int(x.dtype):
    x = mb.cast(x=x, dtype="fp32")

Float inputs behave exactly as before. Integer inputs now lower correctly instead of erroring out.

Both changes are in coremltools/converters/mil/frontend/torch/ops.py, right next to the log/log1p guards they mirror.

Testing

I added test_log10_dtype and test_log2_dtype to TestLog10/TestLog2 in coremltools/converters/mil/frontend/torch/test/test_torch_ops.py, parametrized over np.int32 and np.float32, following the existing test_log_dtype that already covers torch.log on integer input.

I ran them against coremltools 9.0 both ways. Without the fix, the int32 cases fail (the log2/log10 conversion errors on integer input) while float32 passes. With the fix, all 8 selected cases pass (int32 and float32 across the mlprogram and neuralnetwork backends, on CPU, TorchScript frontend).

@TobyRoseman

Copy link
Copy Markdown
Collaborator

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