fix(tests): correct TensorRT-RTX deconv version gate and guard plugin-backed converter tests - #4467
Open
tp5uiuc wants to merge 2 commits into
Open
fix(tests): correct TensorRT-RTX deconv version gate and guard plugin-backed converter tests#4467tp5uiuc wants to merge 2 commits into
tp5uiuc wants to merge 2 commits into
Conversation
TensorRT-RTX defines NV_TENSORRT_MAJOR as TRT_MAJOR_RTX, which is 1. The version comparison guarding setDilationNd()/setNbGroups() therefore fails on every RTX build and selects the pre-TensorRT-7.1 fallback, which rejects any deconvolution with groups > 1 or dilation > 1 and reports "require TensorRT version >= 7.1". TensorRT-RTX supports both, so add the TRT_MAJOR_RTX escape already used for the same purpose in ConversionCtx.cpp and quantization.cpp. This also means setDilationNd() and setNbGroups() are now actually called on RTX rather than silently skipped. Only the TorchScript frontend reaches this code; the dynamo converters are unaffected and already accept grouped deconvolutions on RTX.
The adaptive pooling, aten::norm and aten::instance_norm converters are implemented with TensorRT plugins, which TensorRT-RTX does not provide, so they are already compiled out for RTX builds. The gtests exercising them cannot pass there and fail conversion with "Expected converter to be true but got false". Guard them with #ifndef TRT_MAJOR_RTX, which is visible in these translation units already via tests/util/util.h -> core/ir/ir.h -> NvInfer.h, so no new includes or build dependencies are needed. Non-RTX builds leave TRT_MAJOR_RTX undefined and keep full coverage. Scope per file: - test_pooling.cpp: the 14 adaptive pooling tests; the max/avg pooling tests above them are unaffected. - test_normalize.cpp: the three ATEN_INTERPOLATE_TESTS invocations that emit aten::norm; the frobenius_norm and linalg_norm tests are left enabled since they do not use that converter. - test_instance_norm.cpp: all three tests, which share one aten::instance_norm graph. Two of them additionally carry a pre-existing unconditional GTEST_SKIP() unrelated to RTX.
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.
Description
Fixes 4 failing converter test targets on TensorRT-RTX builds. Two independent causes.
1.
conv_deconv.cpptakes the pre-TensorRT-7.1 fallback on RTX. TensorRT-RTX definesNV_TENSORRT_MAJORasTRT_MAJOR_RTX(== 1), so the version check guardingsetDilationNd()/setNbGroups()is false on every RTX build. Deconvolutions withgroups > 1ordilation > 1are then rejected citing TensorRT 7.1, although RTX supports both. Adds thedefined(TRT_MAJOR_RTX)escape already used for this inConversionCtx.cppandquantization.cpp.Reached only by the TorchScript frontend — the dynamo converter gates strided and dilated deconv on RTX, not groups.
2. Plugin-backed converters have no RTX implementation. Adaptive pooling,
aten::normandaten::instance_normare built on TensorRT plugins that TensorRT-RTX does not provide, and are already compiled out for RTX. Their gtests therefore fail withExpected converter to be true but got false. Guarded with#ifndef TRT_MAJOR_RTX, which is already visible viatests/util/util.h→core/ir/ir.h→NvInfer.h(no new includes or build deps). Non-RTX builds keep full coverage.test_pooling.cpp— 14 adaptive tests guarded; max/avg pooling left enabledtest_normalize.cpp— 6aten::normtests guarded;frobenius_norm/linalg_normleft enabledtest_instance_norm.cpp— all 3 guarded (they share oneaten::instance_normgraph)Verified on TensorRT-RTX 1.6.1.120:
//tests/core/conversion:conversion_testspasses 49/49 targets, includingATenConvTransposeWithGroupConvertsCorrectly, which covers cause 1 and is deliberately left unguarded.Type of change
Checklist: