Repository navigation
Conversation
trt_utils imports tensorrt under a try/except and keeps the answer in TRT_PYTHON_AVAILABLE, which load_onnx_model checks before it reaches get_custom_layers. GraphSanitizer.find_custom_nodes does not, and the onnx extra does not install tensorrt, so autocast on a model holding any non-standard op dies with NameError: name 'trt' is not defined from inside trt_utils rather than saying what is missing. The guard goes in get_custom_layers, where the knowledge lives, so every caller gets the message. Signed-off-by: Dmitry Voropaev <dy.voropaev@gmail.com>
Contributor
This branch has not been deployed
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.
What
modelopt.onnx.autocaston a model that holds any non-standard operator dies withNameError: name 'trt' is not definedinstead of saying that TensorRT is not installed.modelopt/onnx/trt_utils.py:36-41imports TensorRT under atry/except ImportErrorand records the answer:load_onnx_modelchecks it at:414before it goes anywhere nearget_custom_layers.GraphSanitizer.find_custom_nodes(modelopt/onnx/autocast/graphsanitizer.py:136) does not, andget_custom_layersreaches fortrtat its first statement.The
onnxextra inpyproject.tomldoes not listtensorrt, sopip install nvidia-modelopt[onnx]leavesTRT_PYTHON_AVAILABLEfalse, which is exactly the case the flag exists for.Reproduced on this machine, through the public entry point, on a two-node model whose second node sits in a
custom.opsdomain:After the change, the same run says:
CI does not see this because the only test over that path,
test_find_custom_nodes_uses_source_model_pathintests/unit/onnx/autocast/test_graphsanitizer.py:454, mocksget_custom_layers,set_trt_plugin_domainandinfer_types_shapes_tensorrt, so the real TensorRT calls are never made.Why there and not in find_custom_nodes
get_custom_layersis where the knowledge of the dependency lives, so the check covers every caller rather than the one that happens to crash today.infer_types_shapeshas the same dependency but is only reached afterget_custom_layersreturns, so it needs no second guard for this path.An alternative is to make autocast skip the TensorRT half with a warning, matching what
load_onnx_modeldoes at:414. I did not, becauseset_trt_plugin_domainhas already rewritten the custom ops into thetrt.pluginsdomain by then, and carrying on would hand the rest of the pipeline a model with no type or shape information for those nodes. Raising says what to do; skipping would be quieter and less correct. Say if you would rather have the skip and I will send that instead.Test
test_find_custom_nodes_without_tensorrt_says_sobeside the existing custom-node test, withmonkeypatch.setattr(trt_utils, "TRT_PYTHON_AVAILABLE", False), which is the same techniquetests/unit/onnx/quantization/test_quantize_api.py:229already uses. Onmainit fails with theNameErrorabove; after the change it passes.tests/unit/onnx/autocast/test_graphsanitizer.pyis 17 passed.ruff checkandruff format --check(0.15.20) are clean on both files.Unrelated, found next door
While reading
convert.pyI noticedconvert_to_mixed_precisiontakesproviders: list[str] = ["cpu"]and then doesproviders.insert(0, "trt")at:168-170, so the shared default list keeps thetrtentry for every later call in the process that omits the argument. Against the real function object:That is a separate change and I have left it out of this one. Happy to send it if you want it.
Summary by CodeRabbit