Skip to content

Reject inverted RangeDim bounds when upper_bound is positive - #2849

Merged
TobyRoseman merged 1 commit into
apple:mainfrom
rioyu123:codex/fix-rangedim-inverted-bounds
Sep 14, 2026
Merged

TobyRoseman merged 1 commit into
apple:mainfrom
rioyu123:codex/fix-rangedim-inverted-bounds

Conversation

@rioyu123

@rioyu123 rioyu123 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Reject inverted positive finite bounds in RangeDim.__init__ with a ValueError that identifies both bounds, before allocating a symbol.

Currently, this invalid range is accepted when default is omitted:

dim = ct.RangeDim(lower_bound=5, upper_bound=3)
ct.TensorType(name="x", shape=(dim,))
# dim.default is 5, outside the declared upper bound of 3.

A one-op MIL-to-neural-network conversion only rejects it later, during shape processing after conversion passes have run. Checking the bounds at construction reports the configuration error at its source. An inverted range is already rejected during neural-network conversion; this reports it at construction instead. ML Program emission was not exercised (see Testing).

The guard applies only when upper_bound > 0. Equal bounds, lower bound zero, valid finite ranges, and upper_bound=-1 remain supported. Existing non-positive-upper-bound policy and __ior__ are untouched. Tests cover omitted and explicit defaults for inverted ranges, plus the valid cases.

Testing

On Linux / Python 3.12.3, using the source checkout:

  • Final input-type tests against unchanged main (c59d1a2fe535367db7b9b95a8cc2cffaa82dac4d): 3 failed, 11 passed. Omitted default does not raise; explicit defaults raise the older default-validation messages instead of identifying the inverted bounds.
  • With the fix: 14 input-type tests + 9 MIL type tests passed, no skips or xfails.
  • The public ct.RangeDim(lower_bound=5, upper_bound=3) call now raises immediately.
  • git diff --check and compilation of the modified Python files passed.
PYTHONPATH=. python -m pytest -o addopts= -q \
  coremltools/converters/mil/test/test_input_types.py \
  coremltools/converters/mil/mil/tests/test_types.py

These are source-only Python checks, not the full native build/CI suite. macOS inference was not tested. The separate ML Program reproduction could not complete because BlobWriter was unavailable; the late-rejection reproduction above uses the neural-network backend.

@TobyRoseman

Copy link
Copy Markdown
Collaborator

This change looks good.

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

@rioyu123

Copy link
Copy Markdown
Contributor Author

Thanks for the review, @TobyRoseman. I checked the two failed jobs on f6cf4edc:

  • coremltools tests: the Boston Housing download returns HTTP 403, and load_boston() saves the response without checking its status. Reading it then raises pandas.errors.EmptyDataError. This job reports 9 failures and 44 errors; the latest main pipeline's corresponding job shows the same exception and counts.
  • TensorFlow tests: both the initial run and its retry terminate with Fatal Python error: Bus error at about 92%. Both stacks point to TestSpaceToBatchND.test_smoke calling Core ML prediction through _get_predictions in models/model.py. I haven't diagnosed that crash, so I can't yet say whether it's related to this change.

The input/output shape job passed (20 passed, 22 skipped). I've left the patch unchanged. Would you recommend rerunning the TensorFlow job, or is there a particular case you'd like me to investigate further?

@TobyRoseman

Copy link
Copy Markdown
Collaborator

Thanks @rioyu123

@TobyRoseman
TobyRoseman merged commit 2c134ec into apple:main Sep 14, 2026
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