Skip to content

fix(fx): drop the dynamic_range argument from the 8 remaining mark_as_int8_layer calls - #4651

Open
Anai-Guo wants to merge 1 commit into
pytorch:mainfrom
Anai-Guo:fix-fx-mark-as-int8-layer-arity
Open

fix(fx): drop the dynamic_range argument from the 8 remaining mark_as_int8_layer calls#4651
Anai-Guo wants to merge 1 commit into
pytorch:mainfrom
Anai-Guo:fix-fx-mark-as-int8-layer-arity

Conversation

@Anai-Guo

Copy link
Copy Markdown

Problem

13873bd "remove dynamic_range in fx" (#4221) changed

# py/torch_tensorrt/fx/converters/converter_utils.py:710
-def mark_as_int8_layer(layer, dynamic_range):
+def mark_as_int8_layer(layer):

and cleaned up adaptive_avgpool.py, impl/activation.py, maxpool.py,
quantization.py, transformation.py and one of the three sites in
linear.py. Eight call sites were missed and still pass two positional
arguments:

TypeError: mark_as_int8_layer() takes 1 positional argument but 2 were given

Replaying every mark_as_int8_layer(...) call site in fx/converters/ against
the signature parsed out of converter_utils.py on main:

main signature: (layer)

  TypeError add.py:47   (2 positional) -> mark_as_int8_layer() takes 1 positional argument but 2 were given
  TypeError add.py:64   (2 positional) -> ...
  TypeError add.py:70   (2 positional) -> ...
  TypeError batchnorm.py:21  (2 positional) -> ...
  OK        linear.py:27  (1 positional)
  TypeError linear.py:43  (2 positional) -> ...
  TypeError linear.py:51  (2 positional) -> ...
  TypeError mul.py:46   (2 positional) -> ...
  TypeError impl/convolution.py:130  (2 positional) -> ...

8 broken / 1 correct

Each one sits behind an is_quantized / scale is not None guard, so it is
reached by quantized add, add_relu, mul, BatchNorm, Linear and
Conv conversion through the FX path.

Fix

Same idiom #4221 used for linear.py:27: keep the call, drop the argument.
Where the argument was the only consumer of a get_dyn_range(...) result, the
computation and the now-unused import go too.

get_dyn_range itself is kept — quantization.py still uses it.

Note the two shapes #4221 dealt with: sites guarded by if input_val.dynamic_range:
were deleted whole (that attribute is gone in TRT 10), while sites guarded by
if is_quantized: kept the call. All eight sites here are the second kind, so
none of them are deleted.

Checks

black (26.3.1, the pinned pre-commit rev) reports all five files unchanged.
pyflakes is clean on the patched files — the one remaining note,
'typing.Any' imported but unused in impl/convolution.py, is pre-existing on
main and untouched here.

🤖 Generated with Claude Code

…_int8_layer calls

13873bd ("remove dynamic_range in fx", pytorch#4221) changed
mark_as_int8_layer(layer, dynamic_range) to mark_as_int8_layer(layer) and
cleaned up 6 files, but 8 call sites across 5 files still pass two positional
arguments and raise

    TypeError: mark_as_int8_layer() takes 1 positional argument but 2 were given

on every quantized conversion that reaches them: add.py:47/64/70,
batchnorm.py:21, linear.py:43/51, mul.py:46 and impl/convolution.py:130.
linear.py:27, the one site pytorch#4221 did update, is the intended shape.

Follows the same idiom as pytorch#4221: keep the call, drop the argument, and remove
the get_dyn_range() computations and imports that become unused. get_dyn_range
itself is kept -- quantization.py still uses it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant