Support a symbolic batch dim in torch narrow and group_norm - #2808
Open
LeSingh1 wants to merge 1 commit into
Open
Support a symbolic batch dim in torch narrow and group_norm#2808LeSingh1 wants to merge 1 commit into
LeSingh1 wants to merge 1 commit into
Conversation
Both converters materialize x.shape into a constant, which fails when any dim is symbolic: "Cannot add const [is0, 2, 4] ... cannot have symbolic values". narrow built `end = list(x.shape)` and only overwrote the narrowed dim. The end of every other dim is irrelevant, since the slice covers them whole, so state `end = 0` and mask them off instead. That drops the dependency on x.shape entirely. The negative-start handling is unchanged. _group_norm_impl guarded with `any_symbolic(x.shape[2:])`, i.e. it checked only the spatial dims, and then built `[n, num_groups, c // num_groups]` from the batch dim regardless. A symbolic batch took the static path and produced a constant holding a symbol. Widen the guard to the whole shape and read the batch dim from mb.shape in the dynamic path. The existing test_groupnorm_dynamic makes height and width dynamic but pins the batch to 6, which is why only the spatial dims were covered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both converters bake
x.shapeinto a constant, so a symbolic dim aborts the conversion withValueError: Cannot add const [is0, 2, 4] ... cannot have symbolic values.narrowend = list(x.shape), with only the narrowed dim overwritten. The end of every other dim is irrelevant — the slice covers them whole — so stateend = 0and mask them off instead. That drops the dependency onx.shapeentirely. The negative-start handling is unchanged._group_norm_implThe guard was
if not any_symbolic(x.shape[2:]), i.e. it checked only the spatial dims, but the static branch also bakes in the batch dim vianew_shape = [n, num_groups, c // num_groups]. A symbolic batch therefore took the static path and produced a constant holding a symbol. Widened the guard to the whole shape, and the dynamic path now reads the batch dim frommb.shaperather than fromx.shape[0].The existing
test_groupnorm_dynamicmakes height and width dynamic but pins the batch to 6, which is why only the spatial dims were covered.Testing
TestNarrow::test_narrow_dynamic_batchandTestGroupNorm::test_groupnorm_dynamic_batch(ranks 2, 3 and 4, so both the presence and absence of spatial dims are covered). Both fail onmainand pass here; existingTestNarrowandTestGroupNormtests are unchanged and still pass.I could only exercise the TorchScript frontend locally — this machine has a broken scikit-learn install that makes every
TorchFrontend.TORCHEXPORTcase error out, including the pre-existingtest_groupnorm_dynamic, so those results are not mine to report.