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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant