Polygraphy - fix TypeError when converting an engine with polygraphy convert --convert-to trt - #4835
Open
VaggelisGian wants to merge 2 commits into
Open
Polygraphy - fix TypeError when converting an engine with polygraphy convert --convert-to trt#4835VaggelisGian wants to merge 2 commits into
VaggelisGian wants to merge 2 commits into
Conversation
The trt branch of polygraphy convert consumed load_engine_bytes() through a
with statement. That works when the loader produces IHostMemory for a network
input, but when the input is already a serialized engine the loader returns
plain bytes, which do not support the context manager protocol, so commands
like "polygraphy convert model.engine --convert-to trt -o out.plan" always
crashed after doing their work. Assign the result directly; nothing between
the conversion and the end of run_impl needs the prompt release, and Python
reclaims IHostMemory like any other object.
Also correct the load_engine_bytes docstring, which claimed it returns
ICudaEngine, and cover the engine-to-engine path with a CLI-level test.
Test Plan:
cd tools/Polygraphy
PYTHONPATH=$PWD python -m pytest "tests/tools/test_convert.py::TestConvertToTrt::test_engine_to_engine"
fails on main with TypeError: 'bytes' object does not support the context
manager protocol, passes after the change
PYTHONPATH=$PWD python -m pytest tests/tools/test_convert.py::TestConvertToOnnxLikeTrt
7 passed, unchanged
Signed-off-by: Vaggelis <baggelis100@gmail.com>
Upstream appends entries for unreleased changes to the existing undated version section rather than opening a new heading. Signed-off-by: Vaggelis <baggelis100@gmail.com>
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
1 participant
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 does this PR do?
polygraphy convert model.engine --convert-to trt -o out.planalways crashed withTypeError, because the engine input was loaded inside awithstatement intools/Polygraphy/polygraphy/tools/convert/convert.py. Engine inputs return plainbytes, which are not context managers, so every engine-to-engine conversion died before doing any work. The fix assigns the loaded bytes to a plain variable instead.While there, this corrects the
load_engine_bytes()docstring, which claimed it returns anICudaEngine; it actually returns serializedbytes.Testing
New CLI-level regression test
tests/tools/test_convert.py::TestConvertToTrt::test_engine_to_engine: converting a saved.engineback throughpolygraphy convert --convert-to trtmust succeed. It fails on unpatched main with theTypeErrorabove and passes after.Commands and output on this machine:
Full current outcome for the two touched suites on this branch:
The two failures are pre-existing on unpatched
origin/main(verified in a clean worktree of 10d15ae, same 2 failed) and come from TensorRT 11 wheel API drift (ILayer.precision_is_setremoval), unrelated to this change. The new test is the one that flips from red to green.Environment
TensorRT OSS main (10d15ae), polygraphy sources run from the repo.
Python 3.12, tensorrt 11.2.1.2 (pip wheel), polygraphy 0.53.4, onnx 1.17.0,
NVIDIA RTX 5060 Ti, driver 591.86, Windows 11 Pro.
Issue
Per CONTRIBUTING.md, bugfixes start as an issue approved by TensorRT engineers before code review. I could not find an existing issue covering this; happy to file one and link it here if maintainers prefer that flow, or please advise whether this PR can proceed directly.