Skip to content

Preserve RangeDim defaults when forming unbounded unions - #2872

Open
dipeshbabu wants to merge 2 commits into
apple:mainfrom
dipeshbabu:fix/rangedim-unbounded-union-default
Open

dipeshbabu wants to merge 2 commits into
apple:mainfrom
dipeshbabu:fix/rangedim-unbounded-union-default

Conversation

@dipeshbabu

Copy link
Copy Markdown

An in-place union with an unbounded RangeDim treats the -1 upper-bound sentinel as a numeric limit and resets the left-hand default to the merged lower bound. For example, merging RangeDim(2, 10, default=7) with RangeDim(0, -1) changes the default from 7 to 0. Even unioning an unbounded range with itself changes its default. Subsequent Shape construction then uses the wrong default dimension.

Preserve the existing default: a union only widens the accepted range, so a valid default remains valid. This removes the unnecessary clamp without adding allocation or changing bound merging.

Validation:

  • Added 30 regression cases covering finite and unbounded ranges, implicit and explicit defaults, boundary values, disjoint ranges, self-union, object/symbol identity, the right-hand operand, and propagation to Shape.default.
  • Before the fix, 13 of those cases failed.
  • Python 3.10.12 / NumPy 1.26.4 / pytest 7.1.2: 53 tests passed across the regression module, existing input-type tests, and MIL type tests.
  • Regression tests live in coremltools.converters.mil.mil.tests, which the configured GitLab MIL job discovers.
  • git diff --check passed.
  • Full macOS GitLab build/test/documentation CI remains unverified; local validation ran on Windows.

Local test command (pytest file logging/cache disabled because sandbox writes were denied):

python -m pytest coremltools/converters/mil/test/test_input_types.py coremltools/converters/mil/mil/tests/test_range_dim.py coremltools/converters/mil/mil/tests/test_types.py -o addopts= --tb=short -p no:logging -p no:cacheprovider

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant