[OMNIML-5563] Add PETR ONNX PTQ and accuracy evaluation example - #2180
[OMNIML-5563] Add PETR ONNX PTQ and accuracy evaluation example#2180ajrasane wants to merge 2 commits into
Conversation
Signed-off-by: ajrasane <131806219+ajrasane@users.noreply.github.com>
Signed-off-by: ajrasane <131806219+ajrasane@users.noreply.github.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
📝 WalkthroughWalkthroughAdds an end-to-end PETRv1/PETRv2 ONNX PTQ example. It includes container setup, nuScenes metadata and calibration preparation, FP16 conversion, INT8/FP8 quantization, TensorRT evaluation, benchmarking, and documentation. ChangesPETR ONNX PTQ workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant nuScenesDataset
participant PETRPipeline
participant TensorRTRunner
participant CalibrationWriter
nuScenesDataset->>PETRPipeline: provide sampled input data
PETRPipeline->>TensorRTRunner: execute backbone and head engines
TensorRTRunner-->>PETRPipeline: return TensorRT outputs
PETRPipeline->>CalibrationWriter: write ONNX input batches
CalibrationWriter-->>nuScenesDataset: save numbered NPZ batches
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
cjluo-nv
left a comment
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review (bedrock-claude-opus-5) — DM the bot to share feedback.
New self-contained PETRv1/v2 ONNX PTQ example (~990 lines, 12 files) modeled closely on the existing examples/onnx_ptq/far3d/ example. The workflow looks plausible and the author reports full nuScenes validation numbers for all six configurations, so I'm mostly commenting on reuse, conventions, and a couple of silent-failure paths rather than on the quantization results.
Main points:
-
Licensing needs human sign-off (no auto-approve).
evaluate.pyis "Adapted from NVIDIA/DL4AGX .../petr-trt/export_eval" with an OpenMMLab copyright, andprepare_sweep_metadata.pyis adapted from PETRtools/generate_sweep_pkl.pywith a Megvii copyright. Third-party code import plus new third-party pip requirements (mmdet3d/mmcv-full/nuscenes-devkit/lyft-dataset-sdk/pycuda…) should get a maintainer/legal ack. Separately,petr/Dockerfilecarries only the first two SPDX lines instead of the full canonicalLICENSE_HEADERtext thatfar3d/Dockerfileand every other new file in this PR use — please align it. -
convert_to_fp16.pyduplicates a shipped CLI.python -m modelopt.onnx.autocast --onnx_path ... --output_path ... --calibration_data ... --keep_io_types --providers cuda:0 cpualready does exactly what this 43-line script does (seemodelopt/onnx/autocast/__main__.py). Recommend deleting the file and documenting the CLI invocation in the README instead. -
Substantial copy-paste from the FAR3D example.
TRT_TO_TORCH/aligned_tensor/TensorRTRunner/import_plugin/ theeval_kwargsscrub inevaluate.py,FileCalibrationReaderinquantize.py, andfind_backbone_nodes_to_exclude(byte-identical VoVNetOSA4_5+lateral_convsheuristic) all already exist inexamples/onnx_ptq/far3d/. Both examples run in the same Python 3.8 legacy venv layout, so a small shared helper module underexamples/onnx_ptq/would avoid two copies drifting apart. -
Convention regression in
prepare_sweep_metadata.py: all work runs at module import time with nomain()/__main__guard (the siblingfar3d/prepare_metadata.pyuses one), and it silently overwrites an existing output pkl where FAR3D explicitly refuses to. -
Silent-failure risks: calibration input names are taken from ONNX
graph.inputorder but zipped against engine-binding order; and the README tells the reader to run the container-only/opt/petr/bin/pythonbefore the container is built. Details inline.
Also worth noting: this README states TensorRT 11 no longer accepts trtexec --fp16, but far3d/README.md (same 26.07 base image) still instructs --fp16. If that's true, the sibling example's instructions are broken and should be fixed in a follow-up.
No tests, which is consistent with other examples here — fine.
| return parser.parse_args() | ||
|
|
||
|
|
||
| def main(): |
There was a problem hiding this comment.
Bot comment.
This script is a thin re-implementation of the AutoCast CLI that ModelOpt already ships (modelopt/onnx/autocast/__main__.py), which exposes --onnx_path, --output_path, --calibration_data, --keep_io_types, --providers and also calls onnx_utils.save_onnx for you. Suggest dropping this file and putting the equivalent python -m modelopt.onnx.autocast --onnx_path ... --output_path ... --calibration_data ... --keep_io_types --providers cuda:0 cpu command in the README so there's one code path to maintain.
| } | ||
|
|
||
|
|
||
| def find_backbone_nodes_to_exclude(onnx_path): |
There was a problem hiding this comment.
Bot comment.
find_backbone_nodes_to_exclude is the same function as find_encoder_nodes_to_exclude in examples/onnx_ptq/far3d/quantize.py (identical OSA4_5 / lateral_convs VoVNet heuristic); the only difference is that the regex anchoring was moved inside instead of at the call site. Since both examples target the same VoVNet backbone, please factor this into one shared helper rather than keeping two copies — otherwise a future fix to the exclusion heuristic will only land in one example.
| from modelopt.onnx.utils import topologically_sort_graph_nodes | ||
|
|
||
|
|
||
| class FileCalibrationReader(CalibrationDataReader): |
There was a problem hiding this comment.
Bot comment.
FileCalibrationReader here is essentially FileCalibrationReader + DecoderCalibrationReader from far3d/quantize.py merged. Same suggestion as for the node-exclusion helper: extract once under examples/onnx_ptq/ and import from both examples.
| return storage[offset : offset + element_count].view(shape) | ||
|
|
||
|
|
||
| class TensorRTRunner: |
There was a problem hiding this comment.
Bot comment.
TRT_TO_TORCH, aligned_tensor, TensorRTRunner, import_plugin and the eval_kwargs key-scrub loop in main() are near-verbatim copies of far3d/evaluate.py. Given both examples run in the same Python 3.8 legacy venv, a shared examples/onnx_ptq/trt_runner.py (or similar) would remove ~100 duplicated lines.
One behavioral difference worth calling out: this version binds engine inputs positionally (zip(self.input_names, values)) whereas the FAR3D runner binds by tensor name. Positional binding silently produces wrong results if the engine's IO-tensor order ever differs from the caller's list order, and fit_shape will happily squeeze/unsqueeze a mismatched tensor rather than fail. Consider keeping name-based binding, or at least validating each value's shape against self.shapes[name] without the squeeze/unsqueeze fallback.
| if any(self.output_dir.glob("*.npz")): | ||
| raise FileExistsError(f"{self.output_dir} already contains calibration batches") | ||
| graph = onnx.load(onnx_path, load_external_data=False).graph | ||
| self.names = [value.name for value in graph.input] |
There was a problem hiding this comment.
Bot comment.
self.names comes from the ONNX graph.input order, but __call__ receives the buffers in the engine IO-tensor order produced by TensorRTRunner. TensorRT usually preserves the parser's input order, but if it doesn't, the only symptom is calibration .npz files whose keys are attached to the wrong tensors — and quantize.py's reader looks up by name, so this would silently calibrate with swapped inputs (the current len(values) != len(self.names) check won't catch it).
Safer options: pass the engine's input_names (and shapes) into the writer and key the npz off those, or assert each value's shape matches the ONNX input's declared shape before saving.
| return parser.parse_args() | ||
|
|
||
|
|
||
| args = parse_args() |
There was a problem hiding this comment.
Bot comment.
This module executes everything at import time — args = parse_args() plus the NuScenes load and the whole sweep loop live at module scope with no main() / if __name__ == "__main__" guard. The sibling far3d/prepare_metadata.py uses the standard main() pattern; please match it (it also makes the module importable/testable and keeps arg parsing out of import side effects).
Two related nits in the same block:
mmcv.dump(key_infos, info_path)at the end will silently overwrite an existing pkl.far3d/prepare_metadata.pydeliberately raisesFileExistsErrorinstead — worth doing the same here.pickle.load(open(...))leaks the file handle and reintroduces rawpickleeven thoughmmcvis already imported fordump;mmcv.load(path)(or awith open(...)block) is cleaner.
| PETRv2 also needs metadata for the previous camera sweeps: | ||
|
|
||
| ```bash | ||
| /opt/petr/bin/python \ |
There was a problem hiding this comment.
Bot comment.
Ordering problem: this step invokes /opt/petr/bin/python and /opt/Model-Optimizer/examples/onnx_ptq/petr/prepare_sweep_metadata.py, both of which only exist inside the container that's built in the next code block. Either move the sweep-metadata step after the docker build / docker run instructions, or note explicitly that it must be run from inside the container.
Also, the directory layout above lists nuscenes_infos_val.pkl alongside the downloaded nuScenes folders, but that file isn't part of the dataset release — the sweep script depends on it, so please document the mmdetection3d create_data.py (or PETR) step that generates it.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2180 +/- ##
=======================================
Coverage 78.66% 78.66%
=======================================
Files 522 522
Lines 60420 60420
=======================================
Hits 47532 47532
Misses 12888 12888
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 10
🧹 Nitpick comments (4)
examples/onnx_ptq/petr/prepare_calibration.py (1)
103-105: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReconsider the "fork" start method.
set_start_method("fork")raisesRuntimeErrorif a start method is already set in the process. On Linux "fork" is also the default, so the call adds risk without changing behavior.main()initializes CUDA in the parent before the DataLoader forks workers, which makes forking fragile.Remove the call, or pass
force=Trueand add a comment that explains why "fork" is required.♻️ Proposed change
if __name__ == "__main__": - torch.multiprocessing.set_start_method("fork") main()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/onnx_ptq/petr/prepare_calibration.py` around lines 103 - 105, Remove the torch.multiprocessing.set_start_method("fork") call from the __main__ entry point and invoke main() directly, avoiding forced or fragile process-start configuration.examples/onnx_ptq/petr/evaluate.py (2)
234-240: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueCompare the sample count with
>=.
outputs.extendcan add more than one result per batch. If that happens, the==check never matches and the loop runs to the end of the dataset. Use>=.♻️ Proposed change
- if args.max_samples is not None and len(outputs) == args.max_samples: + if args.max_samples is not None and len(outputs) >= args.max_samples: break🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/onnx_ptq/petr/evaluate.py` around lines 234 - 240, Update the max-samples termination condition in the evaluation loop around outputs.extend and args.max_samples to use a greater-than-or-equal comparison, so processing stops when a batch causes outputs to reach or exceed the requested limit.
50-55: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCompute the element count with
math.prod.Line 52 allocates a temporary tensor and calls
.item()only to multiply the shape entries. Usemath.prod(shape)instead. The coding guidelines ask you to avoidtensor.item()and extract Python scalars only when the CPU requires them.♻️ Proposed refactor
+import math + def aligned_tensor(shape, dtype, device, alignment=256): element_size = torch.empty((), dtype=dtype).element_size() - element_count = int(torch.tensor(shape).prod().item()) + element_count = math.prod(shape)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/onnx_ptq/petr/evaluate.py` around lines 50 - 55, Update aligned_tensor to compute element_count with math.prod(shape) instead of constructing a temporary tensor and calling item(); add or reuse the math import as needed while preserving the allocation and alignment behavior.Source: Coding guidelines
examples/onnx_ptq/petr/quantize.py (1)
61-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the exclusion heuristic and warn when it matches nothing.
"OSA4_5"and"lateral_convs"encode PETR VoVNet and FPN layer names. A future export with different node names returns an empty exclusion list, and the accuracy-sensitive nodes are then quantized. The only signal is the count printed at line 110.Add a docstring that states which subgraph the heuristic targets. Raise or warn when the set is empty.
♻️ Proposed change
def find_backbone_nodes_to_exclude(onnx_path): + """Return regex patterns for accuracy-sensitive PETR backbone nodes. + + The heuristic targets the VoVNet ``OSA4_5`` block and every node downstream of the + FPN ``lateral_convs``. It depends on the node names produced by the documented PETR + ONNX export. + """ graph = onnx.load(onnx_path, load_external_data=False).graph @@ + if not excluded: + raise ValueError( + f"No accuracy-sensitive nodes matched in {onnx_path}; check the export node names" + ) return [rf"^{re.escape(name)}$" for name in sorted(excluded)]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/onnx_ptq/petr/quantize.py` around lines 61 - 73, Update find_backbone_nodes_to_exclude with a docstring describing that it targets the PETR VoVNet backbone and FPN lateral-convolution subgraph via the OSA4_5 and lateral_convs node names. Add an explicit empty-result warning or exception before returning when excluded contains no nodes, while preserving the existing regex output for non-empty results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/onnx_ptq/petr/Dockerfile`:
- Line 4: Create a dedicated non-root runtime user in the Dockerfile after
installation steps, grant it access only to the required application and mounted
paths, and set USER to that account before runtime commands. Ensure the existing
installation flow remains privileged while the container shell and example
execution run without root privileges.
- Around line 15-17: Pin all PETR build inputs to immutable artifacts: update
examples/onnx_ptq/petr/Dockerfile lines 15-17 to use pinned uv and Python 3.8
artifacts; add and apply a hashed lock or constraints file covering every
dependency installed through all requirements*.txt files, including transitive
dependencies, at examples/onnx_ptq/petr/requirements.txt lines 4-25; and replace
the default-branch DL4AGX and PETR clones in examples/onnx_ptq/petr/README.md
lines 12-16 with commit checkouts while retaining mmdetection3d v0.17.1 or
pinning it to a commit SHA.
In `@examples/onnx_ptq/petr/evaluate.py`:
- Around line 117-122: Update import_plugin so the plugin import branch reads
plugin_dir via cfg.get before passing it to os.path.dirname, preventing direct
attribute access when plugin is enabled without plugin_dir. Preserve the
existing module path construction for configurations that provide plugin_dir.
In `@examples/onnx_ptq/petr/prepare_calibration.py`:
- Around line 33-47: Update CalibrationWriter.__call__ to bind each calibration
value using the TensorRT engine input names rather than assuming ONNX
graph.input order; pass those names into the writer or validate the engine and
ONNX name sequences before constructing batch. Preserve the existing dtype
conversion while ensuring reordered inputs are detected or correctly labeled.
In `@examples/onnx_ptq/petr/prepare_sweep_metadata.py`:
- Around line 139-149: Update the sweep-building loop around current_cams and
sweep_lists so a camera with an empty "prev" does not index sweep_lists when it
is empty. Preserve the existing reuse of sweep_lists[-1] when prior sweep data
exists, and provide an appropriate empty or partial sweep result for a camera
chain that starts immediately.
- Around line 53-56: Update the pickle loading in the metadata preparation flow
to open the file with a context manager so the handle is closed
deterministically, and add an inline comment documenting that the pickle is
generated locally by the mmdet3d data-preparation step before deserialization.
In `@examples/onnx_ptq/petr/README.md`:
- Around line 92-99: Update both onnxsim invocations in the version loop to use
/opt/petr/bin/python, and document that the AutoCast commands must use the base
environment’s python because it contains Model Optimizer.
In `@examples/onnx_ptq/petr/requirements.txt`:
- Around line 4-6: Update the PETR dependency setup around requirements.txt and
the Dockerfile’s PETR pip install commands by adding a pinned dependency lock
containing direct and transitive packages with hashes, then apply it via pip
constraints or the equivalent lock mechanism to every PETR installation. Ensure
the Docker build does not bypass the lock when disabling PIP_CONSTRAINT, and
keep all PETR installs reproducible.
- Line 18: Update the opencv-python dependency in the PETR requirements to
version 4.8.1.78 or newer, while retaining OpenCV for the MMCV/MMDetection3D
image pipeline and ensuring compatibility with the Python 3.8 NumPy/MMCV stack.
- Line 23: Add an explicit justification for the proprietary
tensorrt-cu13-bindings==11.1.0.106 dependency to the pull request description
and obtain approval from `@NVIDIA/modelopt-setup-codeowners` before merging.
---
Nitpick comments:
In `@examples/onnx_ptq/petr/evaluate.py`:
- Around line 234-240: Update the max-samples termination condition in the
evaluation loop around outputs.extend and args.max_samples to use a
greater-than-or-equal comparison, so processing stops when a batch causes
outputs to reach or exceed the requested limit.
- Around line 50-55: Update aligned_tensor to compute element_count with
math.prod(shape) instead of constructing a temporary tensor and calling item();
add or reuse the math import as needed while preserving the allocation and
alignment behavior.
In `@examples/onnx_ptq/petr/prepare_calibration.py`:
- Around line 103-105: Remove the torch.multiprocessing.set_start_method("fork")
call from the __main__ entry point and invoke main() directly, avoiding forced
or fragile process-start configuration.
In `@examples/onnx_ptq/petr/quantize.py`:
- Around line 61-73: Update find_backbone_nodes_to_exclude with a docstring
describing that it targets the PETR VoVNet backbone and FPN lateral-convolution
subgraph via the OSA4_5 and lateral_convs node names. Add an explicit
empty-result warning or exception before returning when excluded contains no
nodes, while preserving the existing regex output for non-empty results.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d42dabbd-7fe0-4e85-adff-df1f69396135
📒 Files selected for processing (12)
CHANGELOG.rstexamples/onnx_ptq/README.mdexamples/onnx_ptq/petr/Dockerfileexamples/onnx_ptq/petr/README.mdexamples/onnx_ptq/petr/convert_to_fp16.pyexamples/onnx_ptq/petr/evaluate.pyexamples/onnx_ptq/petr/prepare_calibration.pyexamples/onnx_ptq/petr/prepare_sweep_metadata.pyexamples/onnx_ptq/petr/quantize.pyexamples/onnx_ptq/petr/requirements-mmdet3d.txtexamples/onnx_ptq/petr/requirements-torch.txtexamples/onnx_ptq/petr/requirements.txt
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| FROM nvcr.io/nvidia/pytorch:26.07-py3 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Run the container as a non-root user.
The Dockerfile never sets USER, so the runtime shell and example commands run as root. The README mounts host directories into the container. A compromised dependency or example script can then write those bind mounts with container-root privileges. Create a dedicated user after the installation steps, grant access only to the required paths, and set USER before runtime.
Trivy reports DS-0002 for this image.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/onnx_ptq/petr/Dockerfile` at line 4, Create a dedicated non-root
runtime user in the Dockerfile after installation steps, grant it access only to
the required application and mounted paths, and set USER to that account before
runtime commands. Ensure the existing installation flow remains privileged while
the container shell and example execution run without root privileges.
Source: Linters/SAST tools
| RUN python -m pip install --no-cache-dir uv && \ | ||
| uv python install 3.8 && \ | ||
| uv venv --seed --python 3.8 /opt/petr |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files examples/onnx_ptq/petr/Dockerfile examples/onnx_ptq/petr/requirements.txt examples/onnx_ptq/petr/README.md
printf '%s\n' '--- Dockerfile ---'
cat -n examples/onnx_ptq/petr/Dockerfile
printf '%s\n' '--- requirements ---'
cat -n examples/onnx_ptq/petr/requirements.txt
printf '%s\n' '--- README ---'
cat -n examples/onnx_ptq/petr/README.md
printf '%s\n' '--- related PETR references ---'
rg -n -S 'DL4AGX|megvii-research/PETR|mmdetection3d|uv python install|uv venv|requirements\.txt|petr' \
examples/onnx_ptq/petr .github 2>/dev/null | head -250Repository: NVIDIA/Model-Optimizer
Length of output: 18773
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- all PETR requirement files ---'
git ls-files 'examples/onnx_ptq/petr/requirements*.txt' | while read -r file; do
printf '\n### %s\n' "$file"
cat -n "$file"
done
printf '%s\n' '--- source-reference comments and repository metadata ---'
rg -n -S 'Adapted from|Source|Copyright|DL4AGX|PETR|mmdetection3d|nvcr\.io|FROM ' \
examples/onnx_ptq/petr --glob '!README.md'
printf '%s\n' '--- reproducibility-pattern verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
root = Path("examples/onnx_ptq/petr")
dockerfile = (root / "Dockerfile").read_text()
print("Docker base image:", re.search(r"^FROM\s+(\S+)", dockerfile, re.M).group(1))
print("uv install is version-pinned:",
bool(re.search(r"pip install(?:[^\\\n]*\\\n)?[^\\\n]*\buv(?:==|===|~=)", dockerfile)))
print("uv Python selector:", re.search(r"uv python install\s+(\S+)", dockerfile).group(1))
for path in sorted(root.glob("requirements*.txt")):
print(f"\n{path}:")
for lineno, line in enumerate(path.read_text().splitlines(), 1):
line = line.strip()
if not line or line.startswith(("#", "-", "--")):
continue
print(f" {lineno}: {'pinned' if re.search(r'(?<![<>~!])==[^=]', line) else 'unconstrained/ranged'}: {line}")
readme = (root / "README.md").read_text().splitlines()
for lineno, line in enumerate(readme, 1):
if "git clone" in line:
print(f"README:{lineno}: {line.strip()}")
PYRepository: NVIDIA/Model-Optimizer
Length of output: 5019
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- remote source refs ---'
for repo in \
https://github.com/NVIDIA/DL4AGX.git \
https://github.com/megvii-research/PETR.git \
https://github.com/open-mmlab/mmdetection3d.git
do
printf '\n### %s\n' "$repo"
git ls-remote --symref "$repo" HEAD 'refs/tags/v0.17.1' \
| grep -E '^(ref:|[0-9a-f]+\s+HEAD$|[0-9a-f]+\s+refs/tags/v0\.17\.1(\^\{\})?$)' || true
done
printf '%s\n' '--- requirement determinism summary ---'
python3 - <<'PY'
from pathlib import Path
import re
root = Path("examples/onnx_ptq/petr")
for path in sorted(root.glob("requirements*.txt")):
entries = []
for line in path.read_text().splitlines():
line = line.strip()
if not line or line.startswith(("#", "-")):
continue
entries.append(line)
unpinned = [x for x in entries if not re.search(r"(?<![<>~!])==[^=]", x)]
print(f"{path}: {len(entries)} entries, {len(unpinned)} without exact == pins")
if unpinned:
print(" " + "\n ".join(unpinned))
PYRepository: NVIDIA/Model-Optimizer
Length of output: 1070
Pin the PETR build inputs to immutable artifacts.
- Pin
uvand the Python 3.8 artifact inDockerfile#L15-L17. - Add a hashed lock or constraints file for the dependencies installed from all
requirements*.txtfiles.requirements.txtcontains 14 of 22 entries without exact versions, and transitive dependencies remain unresolved. - Replace the default-branch clones for DL4AGX and PETR in
README.md#L12-L13with commit checkouts. Keepmmdetection3dat the reviewedv0.17.1release, or use its commit SHA for strict immutability.
📍 Affects 3 files
examples/onnx_ptq/petr/Dockerfile#L15-L17(this comment)examples/onnx_ptq/petr/requirements.txt#L4-L25examples/onnx_ptq/petr/README.md#L12-L16
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/onnx_ptq/petr/Dockerfile` around lines 15 - 17, Pin all PETR build
inputs to immutable artifacts: update examples/onnx_ptq/petr/Dockerfile lines
15-17 to use pinned uv and Python 3.8 artifacts; add and apply a hashed lock or
constraints file covering every dependency installed through all
requirements*.txt files, including transitive dependencies, at
examples/onnx_ptq/petr/requirements.txt lines 4-25; and replace the
default-branch DL4AGX and PETR clones in examples/onnx_ptq/petr/README.md lines
12-16 with commit checkouts while retaining mmdetection3d v0.17.1 or pinning it
to a commit SHA.
| def import_plugin(cfg): | ||
| if cfg.get("custom_imports"): | ||
| import_modules_from_strings(**cfg.custom_imports) | ||
| if cfg.get("plugin"): | ||
| plugin_dir = os.path.dirname(cfg.plugin_dir).split("/") | ||
| importlib.import_module(".".join(plugin_dir)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle a config that sets plugin without plugin_dir.
Line 120 checks cfg.get("plugin"), and line 121 then reads cfg.plugin_dir. If a config enables plugin but omits plugin_dir, the access raises AttributeError. Read plugin_dir through cfg.get.
🐛 Proposed fix
- if cfg.get("plugin"):
- plugin_dir = os.path.dirname(cfg.plugin_dir).split("/")
- importlib.import_module(".".join(plugin_dir))
+ plugin_dir = cfg.get("plugin_dir")
+ if cfg.get("plugin") and plugin_dir:
+ importlib.import_module(".".join(os.path.dirname(plugin_dir).split("/")))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def import_plugin(cfg): | |
| if cfg.get("custom_imports"): | |
| import_modules_from_strings(**cfg.custom_imports) | |
| if cfg.get("plugin"): | |
| plugin_dir = os.path.dirname(cfg.plugin_dir).split("/") | |
| importlib.import_module(".".join(plugin_dir)) | |
| def import_plugin(cfg): | |
| if cfg.get("custom_imports"): | |
| import_modules_from_strings(**cfg.custom_imports) | |
| plugin_dir = cfg.get("plugin_dir") | |
| if cfg.get("plugin") and plugin_dir: | |
| importlib.import_module(".".join(os.path.dirname(plugin_dir).split("/"))) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/onnx_ptq/petr/evaluate.py` around lines 117 - 122, Update
import_plugin so the plugin import branch reads plugin_dir via cfg.get before
passing it to os.path.dirname, preventing direct attribute access when plugin is
enabled without plugin_dir. Preserve the existing module path construction for
configurations that provide plugin_dir.
| graph = onnx.load(onnx_path, load_external_data=False).graph | ||
| self.names = [value.name for value in graph.input] | ||
| self.dtypes = [ | ||
| onnx.helper.tensor_dtype_to_np_dtype(value.type.tensor_type.elem_type) | ||
| for value in graph.input | ||
| ] | ||
| self.saved = 0 | ||
|
|
||
| def __call__(self, values): | ||
| if len(values) != len(self.names): | ||
| raise ValueError(f"Received {len(values)} inputs, expected {len(self.names)}") | ||
| batch = { | ||
| name: value.detach().cpu().numpy().astype(dtype, copy=False) | ||
| for name, dtype, value in zip(self.names, self.dtypes, values) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Compare ONNX graph input order with the TensorRT input name order used by the writer.
set -euo pipefail
rg -n -C5 'input_names|get_tensor_name|get_tensor_mode' examples/onnx_ptq/petr/evaluate.py
rg -n -C5 'input_callback|graph.input' examples/onnx_ptq/petr
rg -n -C5 'trtexec|onnx_export|export' examples/onnx_ptq/petr/README.mdRepository: NVIDIA/Model-Optimizer
Length of output: 11108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- prepare_calibration.py ---'
cat -n examples/onnx_ptq/petr/prepare_calibration.py | sed -n '1,130p'
printf '%s\n' '--- evaluate.py ---'
cat -n examples/onnx_ptq/petr/evaluate.py | sed -n '58,125p'
printf '%s\n' '--- call sites ---'
rg -n -C8 'CalibrationWriter|TensorRTRunner\(' examples/onnx_ptq/petr
printf '%s\n' '--- quantize.py ---'
cat -n examples/onnx_ptq/petr/quantize.py | sed -n '1,90p'Repository: NVIDIA/Model-Optimizer
Length of output: 16908
🌐 Web query:
site:docs.nvidia.com TensorRT get_tensor_name index input order ONNX parser network inputs order guarantee
💡 Result:
TensorRT does not provide a formal guarantee that the order of inputs or outputs in the parsed engine will match the order defined in the original ONNX model. Because TensorRT's internal graph optimization process can reorder or optimize tensors, developers should not rely on index-based access for input and output management [1][2]. In modern TensorRT (versions 10.x and later), the recommended approach is to use the name-based API rather than index-based bindings [1][2]. You should iterate through the engine using the number of total I/O tensors and retrieve the specific tensor name to interact with it, rather than assuming a fixed order [1][2]: 1. Use engine.num_io_tensors to determine the total count of I/O tensors [1]. 2. Use engine.get_tensor_name(index) to get the string name of each tensor [3][1]. 3. Use engine.get_tensor_mode(tensor_name) to confirm whether a specific tensor is an INPUT or an OUTPUT [1][2]. 4. Use engine.set_tensor_address(tensor_name, address) to bind your buffers using the tensor's name instead of a numeric index [1][2]. By relying on tensor names rather than indices, you ensure your code remains robust against changes in the model's graph structure or potential reordering by the ONNX parser and engine builder [1].
Citations:
- 1: https://docs.nvidia.com/deeplearning/tensorrt/11.1.0/api/migration/tensorrt-8x-to-10x-python-api-patterns.html
- 2: https://docs.nvidia.com/deeplearning/tensorrt/10.x.x/api/tensorrt-8x-to-10x-python-api.html
- 3: https://docs.nvidia.com/deeplearning/tensorrt/latest/_static/python-api/infer/Core/Engine.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- order-related assertions and name handoff ---'
rg -n -i -C4 'graph\.input|input_names|input name|tensor_name|TensorIOMode|engine.*order|order.*engine' examples/onnx_ptq/petr
printf '%s\n' '--- PETRPipeline runner argument flow ---'
cat -n examples/onnx_ptq/petr/evaluate.py | sed -n '145,225p'
printf '%s\n' '--- ONNX graph input ordering references ---'
rg -n -i -C3 'input.*order|order.*input|input_names|graph\.input' examples README.md docs 2>/dev/null || trueRepository: NVIDIA/Model-Optimizer
Length of output: 20574
Bind calibration values by TensorRT tensor name.
TensorRT does not guarantee that engine I/O order matches ONNX graph.input order. CalibrationWriter currently labels positional values with ONNX names, so a reordered engine can produce incorrectly named calibration data without detection. Pass engine input names to the writer or validate both name sequences before writing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/onnx_ptq/petr/prepare_calibration.py` around lines 33 - 47, Update
CalibrationWriter.__call__ to bind each calibration value using the TensorRT
engine input names rather than assuming ONNX graph.input order; pass those names
into the writer or validate the engine and ONNX name sequences before
constructing batch. Preserve the existing dtype conversion while ensuring
reordered inputs are detected or correctly labeled.
| info_path = args.output or os.path.join(data_root, f"mmdet3d_nuscenes_30f_infos_{info_prefix}.pkl") | ||
| key_infos = pickle.load(open(os.path.join(data_root, f"nuscenes_infos_{info_prefix}.pkl"), "rb")) | ||
| nuscenes_version = "v1.0-test" if info_prefix == "test" else "v1.0-trainval" | ||
| nuscenes = NuScenes(nuscenes_version, data_root) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Close the file handle and justify the pickle load.
pickle.load(open(...)) leaks the file object until garbage collection. It also deserializes a metadata file that SECURITY.md treats as potentially untrusted.
Use a context manager. Add an inline comment that states the pickle is generated locally by the mmdet3d data-preparation step, as SECURITY.md requires for any deserialization exception.
🔒 Proposed fix
-key_infos = pickle.load(open(os.path.join(data_root, f"nuscenes_infos_{info_prefix}.pkl"), "rb"))
+# Safe: this pickle is produced locally by the mmdet3d nuScenes data-preparation step
+# documented in README.md, not supplied by an external user.
+with open(os.path.join(data_root, f"nuscenes_infos_{info_prefix}.pkl"), "rb") as info_file:
+ key_infos = pickle.load(info_file)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| info_path = args.output or os.path.join(data_root, f"mmdet3d_nuscenes_30f_infos_{info_prefix}.pkl") | |
| key_infos = pickle.load(open(os.path.join(data_root, f"nuscenes_infos_{info_prefix}.pkl"), "rb")) | |
| nuscenes_version = "v1.0-test" if info_prefix == "test" else "v1.0-trainval" | |
| nuscenes = NuScenes(nuscenes_version, data_root) | |
| info_path = args.output or os.path.join(data_root, f"mmdet3d_nuscenes_30f_infos_{info_prefix}.pkl") | |
| # Safe: this pickle is produced locally by the mmdet3d nuScenes data-preparation step | |
| # documented in README.md, not supplied by an external user. | |
| with open(os.path.join(data_root, f"nuscenes_infos_{info_prefix}.pkl"), "rb") as info_file: | |
| key_infos = pickle.load(info_file) | |
| nuscenes_version = "v1.0-test" if info_prefix == "test" else "v1.0-trainval" | |
| nuscenes = NuScenes(nuscenes_version, data_root) |
🧰 Tools
🪛 ast-grep (0.45.1)
[warning] 53-53: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(os.path.join(data_root, f"nuscenes_infos_{info_prefix}.pkl"), "rb")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 53-53: pickle.load/loads executes arbitrary code when the data is untrusted (a model file, cache, or request payload). Use a safe format like JSON, or only unpickle data from a trusted, integrity-checked source.
Context: pickle.load(open(os.path.join(data_root, f"nuscenes_infos_{info_prefix}.pkl"), "rb"))
Note: [CWE-502] Deserialization of Untrusted Data.
(pickle-deserialization-python)
🪛 OpenGrep (1.26.0)
[ERROR] 54-54: pickle.load/loads deserializes arbitrary Python objects and can execute arbitrary code. Use a safe format like JSON instead.
(coderabbit.deserialization.python-pickle)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/onnx_ptq/petr/prepare_sweep_metadata.py` around lines 53 - 56,
Update the pickle loading in the metadata preparation flow to open the file with
a context manager so the handle is closed deterministically, and add an inline
comment documenting that the pickle is generated locally by the mmdet3d
data-preparation step before deserialization.
Sources: Path instructions, Linters/SAST tools
| for _ in range(num_sweep): | ||
| sweep_cams = {} | ||
| for cam in sensors: | ||
| if current_cams[cam]["prev"] == "": | ||
| sweep_cams = sweep_lists[-1] | ||
| break | ||
| sample_data = nuscenes.get("sample_data", current_cams[cam]["prev"]) | ||
| sweep_cam = add_frame(sample_data, e2g_t, l2e_t, l2e_r_mat, e2g_r_mat) | ||
| current_cams[cam] = sample_data | ||
| sweep_cams[cam] = sweep_cam | ||
| sweep_lists.append(sweep_cams) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard sweep_lists[-1] against an empty list.
Line 143 reads sweep_lists[-1] when a camera record has no prev. On the first sweep iteration sweep_lists is still empty, so this raises IndexError. The outer guard at line 137 only checks sample["prev"], not each camera chain, so a scene-start sample with unequal camera sweep counts aborts the whole metadata generation.
🐛 Proposed fix
for cam in sensors:
if current_cams[cam]["prev"] == "":
- sweep_cams = sweep_lists[-1]
+ sweep_cams = sweep_lists[-1] if sweep_lists else {}
break📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for _ in range(num_sweep): | |
| sweep_cams = {} | |
| for cam in sensors: | |
| if current_cams[cam]["prev"] == "": | |
| sweep_cams = sweep_lists[-1] | |
| break | |
| sample_data = nuscenes.get("sample_data", current_cams[cam]["prev"]) | |
| sweep_cam = add_frame(sample_data, e2g_t, l2e_t, l2e_r_mat, e2g_r_mat) | |
| current_cams[cam] = sample_data | |
| sweep_cams[cam] = sweep_cam | |
| sweep_lists.append(sweep_cams) | |
| for _ in range(num_sweep): | |
| sweep_cams = {} | |
| for cam in sensors: | |
| if current_cams[cam]["prev"] == "": | |
| sweep_cams = sweep_lists[-1] if sweep_lists else {} | |
| break | |
| sample_data = nuscenes.get("sample_data", current_cams[cam]["prev"]) | |
| sweep_cam = add_frame(sample_data, e2g_t, l2e_t, l2e_r_mat, e2g_r_mat) | |
| current_cams[cam] = sample_data | |
| sweep_cams[cam] = sweep_cam | |
| sweep_lists.append(sweep_cams) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/onnx_ptq/petr/prepare_sweep_metadata.py` around lines 139 - 149,
Update the sweep-building loop around current_cams and sweep_lists so a camera
with an empty "prev" does not index sweep_lists when it is empty. Preserve the
existing reuse of sweep_lists[-1] when prior sweep data exists, and provide an
appropriate empty or partial sweep result for a camera chain that starts
immediately.
| for version in v1 v2; do | ||
| model="PETR${version}" | ||
| python -m onnxsim \ | ||
| "onnx_files/${model}.extract_feat.onnx" \ | ||
| "onnx_files/sim_${model}.extract_feat.onnx" | ||
| python -m onnxsim \ | ||
| "onnx_files/${model}.pts_bbox_head.forward.onnx" \ | ||
| "onnx_files/sim_${model}.pts_bbox_head.forward.onnx" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(README\.md|Dockerfile|requirements\.txt)$|petr'
printf '%s\n' '--- README context ---'
sed -n '70,165p' examples/onnx_ptq/petr/README.md
printf '%s\n' '--- Dockerfile candidates and context ---'
fd -i '^dockerfile$' . | while IFS= read -r f; do
printf '\n### %s\n' "$f"
rg -n -C 5 'petr|requirements|onnxsim|PATH|autocast|model optimizer|mo' "$f" || true
done
printf '%s\n' '--- requirements context ---'
fd -i '^requirements\.txt$' . | while IFS= read -r f; do
printf '\n### %s\n' "$f"
rg -n -C 3 'onnxsim|onnx|openvino|mo|model' "$f" || true
done
printf '%s\n' '--- all relevant command references ---'
rg -n -C 4 'onnxsim|AutoCast|autocast|/opt/petr/bin/python|python -m' examples/onnx_ptq/petr .github docker* 2>/dev/null || trueRepository: NVIDIA/Model-Optimizer
Length of output: 30539
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- complete PETR Dockerfile ---'
cat -n examples/onnx_ptq/petr/Dockerfile
printf '%s\n' '--- README setup and AutoCast sections ---'
sed -n '1,70p' examples/onnx_ptq/petr/README.md
sed -n '136,165p' examples/onnx_ptq/petr/README.md
printf '%s\n' '--- modelopt ONNX dependency and AutoCast declarations ---'
rg -n -C 5 'onnx|autocast|convert_to_mixed_precision|modelopt\.onnx' pyproject.toml setup.cfg setup.py modelopt examples/onnx_ptq/petr 2>/dev/null || true
printf '%s\n' '--- read-only command/environment verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
docker = Path("examples/onnx_ptq/petr/Dockerfile").read_text()
readme = Path("examples/onnx_ptq/petr/README.md").read_text()
requirements = Path("examples/onnx_ptq/petr/requirements.txt").read_text()
docker_path_lines = [
line for line in docker.splitlines()
if re.search(r'(^|\s)(ENV|ARG).*PATH|PATH\s*=', line)
]
python_invocations = [
(i, line.strip())
for i, line in enumerate(readme.splitlines(), 1)
if re.search(r'(^|\s)(python|/opt/petr/bin/python)\b', line)
]
print("Dockerfile PATH declarations:", docker_path_lines or "<none>")
print("requirements.txt declares onnxsim:", bool(re.search(r"(?m)^onnxsim\s*$", requirements)))
print("Dockerfile installs PETR requirements with /opt/petr/bin/python:",
"/opt/petr/bin/python -m pip install" in docker and
"-r /tmp/petr-requirements/requirements.txt" in docker)
print("Dockerfile installs Model Optimizer in base-environment python:",
'env -u PIP_CONSTRAINT python -m pip install' in docker)
print("README Python invocations:")
for item in python_invocations:
print(" ", item)
PYRepository: NVIDIA/Model-Optimizer
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -eu
log=/tmp/coderabbit-shell-logs/shell-output-yUgE4z
printf '%s\n' '--- Dockerfile and README excerpts from prior result ---'
rg -n -A 4 -B 2 '^ *([0-9]+:)?(FROM|ENV|RUN|COPY)|/opt/petr/bin/python|python /opt/Model-Optimizer/examples/onnx_ptq/petr/convert_to_fp16.py|python -m onnxsim|Docker|container|PATH' "$log" | head -n 240
printf '%s\n' '--- verifier output from prior result ---'
rg -n -A 30 -B 3 'Dockerfile PATH declarations|requirements.txt declares|README Python invocations' "$log"
printf '%s\n' '--- focused dependency declarations ---'
rg -n -A 8 -B 4 'onnxsim|optional-dependencies|onnx.*=' pyproject.toml setup.cfg setup.py 2>/dev/null | head -n 180
printf '%s\n' '--- conversion script imports ---'
cat -n examples/onnx_ptq/petr/convert_to_fp16.py | sed -n '1,80p'Repository: NVIDIA/Model-Optimizer
Length of output: 17961
Use the correct Python environment for each command.
Use /opt/petr/bin/python -m onnxsim for both simplification commands. Document that the AutoCast commands use the base environment's python, where Model Optimizer is installed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/onnx_ptq/petr/README.md` around lines 92 - 99, Update both onnxsim
invocations in the version loop to use /opt/petr/bin/python, and document that
the AutoCast commands must use the base environment’s python because it contains
Model Optimizer.
| einops | ||
| ipython<9 | ||
| lyft-dataset-sdk |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- PETR requirement files ---'
git ls-files '*petr*' '*requirements*.txt' | sed -n '1,160p'
printf '%s\n' '--- requirements file ---'
cat -n examples/onnx_ptq/petr/requirements.txt
printf '%s\n' '--- PETR and Docker references ---'
rg -n -i 'petr|requirements\.txt|docker|reproducible|lock|constraint' examples/onnx_ptq .github Dockerfile* 2>/dev/null | sed -n '1,240p'
printf '%s\n' '--- repository diff summary ---'
git diff --statRepository: NVIDIA/Model-Optimizer
Length of output: 20845
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- PETR Dockerfile ---'
cat -n examples/onnx_ptq/petr/Dockerfile
printf '%s\n' '--- PETR auxiliary requirements ---'
for f in examples/onnx_ptq/petr/requirements-torch.txt examples/onnx_ptq/petr/requirements-mmdet3d.txt; do
echo "--- $f ---"
cat -n "$f"
done
printf '%s\n' '--- constraint and lock references ---'
rg -n -i 'PIP_CONSTRAINT|constraints|uv\.lock|pip-compile|--require-hashes|hashes' \
examples/onnx_ptq/petr pyproject.toml uv.lock .github 2>/dev/null | sed -n '1,220p'
printf '%s\n' '--- all PETR requirement specifiers ---'
python3 - <<'PY'
from pathlib import Path
from packaging.requirements import Requirement
root = Path("examples/onnx_ptq/petr")
for path in sorted(root.glob("requirements*.txt")):
print(f"--- {path} ---")
for raw in path.read_text().splitlines():
line = raw.strip()
if not line or line.startswith(("#", "-", "git+", "http:")):
continue
try:
req = Requirement(line)
print(f"{req.name}: {req.specifier or '<unconstrained>'}")
except Exception:
print(f"unparsed: {line}")
PYRepository: NVIDIA/Model-Optimizer
Length of output: 4420
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
root = Path("examples/onnx_ptq/petr")
unconstrained = []
for path in sorted(root.glob("requirements*.txt")):
print(f"--- {path} ---")
for number, raw in enumerate(path.read_text().splitlines(), 1):
line = raw.strip()
if not line or line.startswith("#") or line.startswith("-"):
continue
match = re.match(r"([A-Za-z0-9_.-]+)(.*)$", line)
if not match:
print(f"{number}: unparsed: {line}")
continue
name, specifier = match.groups()
specifier = specifier.strip()
print(f"{number}: {name}: {specifier or '<unconstrained>'}")
if not specifier:
unconstrained.append(f"{path}:{number}:{name}")
print("--- unconstrained requirements ---")
print("\n".join(unconstrained) or "<none>")
PY
printf '%s\n' '--- PETR pip install safeguards ---'
awk '
/pip install/ {in_install=1}
in_install {print NR ":" $0}
in_install && /&&/ {in_install=0}
' examples/onnx_ptq/petr/Dockerfile
if rg -n -- '--constraint|--require-hashes|constraints|requirements.*lock' \
examples/onnx_ptq/petr 2>/dev/null; then
:
else
echo '<no PETR constraints, lock files, or hash enforcement found>'
fiRepository: NVIDIA/Model-Optimizer
Length of output: 2315
Add and use a pinned PETR dependency lock.
The Dockerfile disables PIP_CONSTRAINT and installs PETR requirements without a constraints or lock file. Pin the direct and transitive dependencies, including hashes, and apply the lock to every PETR pip install command so clean builds remain reproducible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/onnx_ptq/petr/requirements.txt` around lines 4 - 6, Update the PETR
dependency setup around requirements.txt and the Dockerfile’s PETR pip install
commands by adding a pinned dependency lock containing direct and transitive
packages with hashes, then apply it via pip constraints or the equivalent lock
mechanism to every PETR installation. Ensure the Docker build does not bypass
the lock when disabling PIP_CONSTRAINT, and keep all PETR installs reproducible.
| onnx-graphsurgeon==0.6.1 | ||
| onnxruntime | ||
| onnxsim | ||
| opencv-python==4.5.5.64 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- requirement file ---'
cat -n examples/onnx_ptq/petr/requirements.txt
printf '%s\n' '--- related project files ---'
fd -t f . examples/onnx_ptq/petr examples/onnx_ptq | head -200
printf '%s\n' '--- cv2 usage ---'
rg -n --glob '*.py' --glob '*.ipynb' --glob '*.md' '(^|[^[:alnum:]_])cv2([^[:alnum:]_]|$)|opencv' examples/onnx_ptq/petr examples/onnx_ptq || true
printf '%s\n' '--- Python and dependency constraints ---'
rg -n --glob 'requirements*.txt' --glob 'pyproject.toml' --glob 'setup*.py' --glob '*.yml' --glob '*.yaml' 'python_requires|Python 3\.8|numpy|mmcv|opencv' . | head -300Repository: NVIDIA/Model-Optimizer
Length of output: 4401
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- PETR Docker and documentation ---'
cat -n examples/onnx_ptq/petr/Dockerfile
cat -n examples/onnx_ptq/petr/README.md | sed -n '1,220p'
printf '%s\n' '--- PETR Python imports ---'
rg -n '^(from|import) |cv2|mmcv|mmdet|mmseg' examples/onnx_ptq/petr --glob '*.py'
printf '%s\n' '--- shared ONNX PTQ image/data code imports ---'
rg -n 'cv2|opencv|mmcv|mmdet|mmseg' examples/onnx_ptq --glob '*.py' --glob '*.txt' --glob 'Dockerfile*'
printf '%s\n' '--- related dependency files ---'
cat -n examples/onnx_ptq/petr/requirements-mmdet3d.txt
cat -n examples/onnx_ptq/petr/requirements-torch.txtRepository: NVIDIA/Model-Optimizer
Length of output: 18422
🌐 Web query:
opencv-python 4.5.5.64 bundled libwebp version CVE-2023-4863 fixed release Python 3.8 compatibility
💡 Result:
The opencv-python version 4.5.5.64 is vulnerable to CVE-2023-4863 because it bundles an older, affected version of the libwebp library [1][2][3]. This vulnerability involves a heap-based buffer overflow in libwebp that was addressed in opencv-python starting with version 4.8.1.78, which upgraded the bundled libwebp to version 1.3.2 [1][3][4]. Regarding Python 3.8 compatibility, opencv-python version 4.5.5.64 lists Python >= 3.6 as a requirement [5], meaning it is compatible with Python 3.8. While later versions of opencv-python may eventually drop support for older Python versions, official changelogs and advisory records for the 4.8.1.78 update do not indicate a break in Python 3.8 compatibility. If you are using Python 3.8, it is strongly recommended to update to at least version 4.8.1.78 or a newer stable release to mitigate this critical vulnerability [1][6].
Citations:
- 1: https://osv.dev/vulnerability/GHSA-qr4w-53vh-m672
- 2: https://osv.dev/vulnerability/PYSEC-2023-183
- 3: GHSA-qr4w-53vh-m672
- 4: https://github.com/pypa/advisory-database/blob/main/vulns/opencv-python/PYSEC-2023-183.yaml
- 5: https://pypi.org/project/opencv-python/4.5.5.64/
- 6: https://security.snyk.io/vuln/SNYK-PYTHON-OPENCVPYTHON-5926695
🌐 Web query:
PyPI opencv-python release history Python 3.8 support 4.8.1.78 4.10.0.84
💡 Result:
The opencv-python package provides pre-built wheels that include support for Python 3.8 [1][2]. Support for Python 3.8 was introduced in late 2019 [3][4]. Regarding the specific versions requested: - opencv-python version 4.8.1.78 was released on September 27, 2023 [5][6]. - opencv-python version 4.10.0.84 was released on January 16, 2025 [7][5]. Both of these versions include support for Python 3.8, as the package provides pre-built wheels for officially supported, non-EOL Python versions [8][9][1][2]. While Python 3.8 has reached its end-of-life, the repository continues to list it among the supported versions for its pre-built wheels [1][2].
Citations:
- 1: https://github.com/opencv/opencv-python/
- 2: https://pypi.org/project/opencv-python/
- 3: python3.8 is out opencv/opencv-python#253
- 4: Opencv-python for python 3.8 opencv/opencv#15952
- 5: https://github.com/opencv/opencv-python/releases
- 6: https://github.com/opencv/opencv-python/releases/tag/78
- 7: https://github.com/opencv/opencv-python/releases/tag/84
- 8: https://pypi.org/project/opencv-python/4.10.0.84/
- 9: https://pypi.org/project/opencv-python/4.8.1.78/
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
import urllib.request
from email.parser import Parser
def get(url):
with urllib.request.urlopen(url, timeout=20) as response:
return json.load(response)
print("--- OSV record ---")
osv = get("https://api.osv.dev/v1/vulns/GHSA-qr4w-53vh-m672")
print("id:", osv.get("id"))
print("summary:", osv.get("summary"))
for affected in osv.get("affected", []):
print("package:", affected.get("package"))
print("ranges:", json.dumps(affected.get("ranges"), separators=(",", ":")))
print("database_specific:", json.dumps(affected.get("database_specific"), separators=(",", ":")))
print("--- PyPI release metadata ---")
for version in ("4.5.5.64", "4.8.1.78", "4.10.0.84"):
data = get(f"https://pypi.org/pypi/opencv-python/{version}/json")
info = data["info"]
print(version, "requires_python=", info.get("requires_python"))
files = [
(f["filename"], f.get("requires_python"))
for f in data["releases"].get(version, [])
]
print("files:", json.dumps(files))
PYRepository: NVIDIA/Model-Optimizer
Length of output: 2499
Replace the vulnerable OpenCV wheel.
opencv-python==4.5.5.64 bundles vulnerable libwebp, including CVE-2023-4863. Upgrade to at least opencv-python==4.8.1.78 and validate it with the Python 3.8 NumPy/MMCV stack. Keep the dependency because the PETR runtime uses image pipelines from MMCV/MMDetection3D.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/onnx_ptq/petr/requirements.txt` at line 18, Update the opencv-python
dependency in the PETR requirements to version 4.8.1.78 or newer, while
retaining OpenCV for the MMCV/MMDetection3D image pipeline and ensuring
compatibility with the Python 3.8 NumPy/MMCV stack.
Sources: MCP tools, Linters/SAST tools
| pycuda | ||
| scikit-image | ||
| setuptools<81 | ||
| tensorrt-cu13-bindings==11.1.0.106 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- requirement context ---'
file=$(git ls-files | grep -F 'examples/onnx_ptq/petr/requirements.txt' | head -n 1)
[ -n "$file" ] && { nl -ba "$file" | sed -n '15,27p'; git diff -- "$file"; }
printf '%s\n' '--- repository guidance ---'
rg -n -i 'modelopt-setup-codeowners|proprietary|requirements|license' CONTRIBUTING.md SECURITY.md pyproject.toml 2>/dev/null | head -80 || true
printf '%s\n' '--- PyPI metadata ---'
curl -fsSL 'https://pypi.org/pypi/tensorrt-cu13-bindings/11.1.0.106/json' |
python3 -c 'import json,sys; d=json.load(sys.stdin); print("name:",d["info"]["name"]); print("version:",d["info"]["version"]); print("license:",repr(d["info"].get("license"))); print("classifiers:", [x for x in d["info"].get("classifiers",[]) if "License" in x]); print("requires_python:",d["info"].get("requires_python"))'Repository: NVIDIA/Model-Optimizer
Length of output: 5024
🏁 Script executed:
#!/bin/bash
set -e
file='examples/onnx_ptq/petr/requirements.txt'
printf '%s\n' '--- requirement context ---'
awk 'NR>=15 && NR<=27 {printf "%6d\t%s\n", NR, $0}' "$file"
printf '%s\n' '--- diff for the reviewed file ---'
git diff --no-ext-diff -- "$file"
printf '%s\n' '--- staged diff for the reviewed file ---'
git diff --cached --no-ext-diff -- "$file"Repository: NVIDIA/Model-Optimizer
Length of output: 509
🏁 Script executed:
#!/bin/bash
set -e
file='examples/onnx_ptq/petr/requirements.txt'
parent=$(git rev-parse HEAD^)
printf 'HEAD: %s\nPARENT: %s\n' "$(git rev-parse HEAD)" "$parent"
printf '%s\n' '--- parent-to-HEAD diff for the reviewed file ---'
git diff --no-ext-diff "$parent" HEAD -- "$file"
printf '%s\n' '--- parent version of the dependency line ---'
git show "$parent:$file" 2>/dev/null | awk '/tensorrt-cu13-bindings/ {print}'Repository: NVIDIA/Model-Optimizer
Length of output: 387
🏁 Script executed:
#!/bin/bash
set -e
file='examples/onnx_ptq/petr/requirements.txt'
printf '%s\n' '--- refs ---'
git branch --all --no-color
printf '%s\n' '--- recent commits touching the file ---'
git log --all --oneline --decorate -n 12 -- "$file"
printf '%s\n' '--- current commit summary ---'
git show --no-ext-diff --format='%H%n%P%n%s' --stat --oneline HEAD -- "$file"Repository: NVIDIA/Model-Optimizer
Length of output: 374
🏁 Script executed:
#!/bin/bash
set -e
file='examples/onnx_ptq/petr/requirements.txt'
base=$(git merge-base HEAD main)
printf 'merge-base: %s\n' "$base"
printf '%s\n' '--- branch diff for the reviewed file ---'
git diff --no-ext-diff "$base"...HEAD -- "$file"
printf '%s\n' '--- file presence at merge-base ---'
if git cat-file -e "$base:$file" 2>/dev/null; then
printf 'present\n'
git show "$base:$file" | awk '/tensorrt-cu13-bindings/ {print}'
else
printf 'absent\n'
fiRepository: NVIDIA/Model-Optimizer
Length of output: 980
Obtain code-owner approval for the proprietary TensorRT binding.
Because tensorrt-cu13-bindings==11.1.0.106 is a new dependency and PyPI classifies it as Proprietary, add an explicit justification to the PR description and obtain approval from @NVIDIA/modelopt-setup-codeowners before merge.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/onnx_ptq/petr/requirements.txt` at line 23, Add an explicit
justification for the proprietary tensorrt-cu13-bindings==11.1.0.106 dependency
to the pull request description and obtain approval from
`@NVIDIA/modelopt-setup-codeowners` before merging.
Sources: Coding guidelines, MCP tools
There was a problem hiding this comment.
@claude summarize all new dependency licenses not included in pyproject.toml and report a table of dependency with their License name
There was a problem hiding this comment.
Claude finished @kevalmorabia97's task in 2m 38s —— View job
New dependency license summary
Scope: every package added by this PR in examples/onnx_ptq/petr/requirements.txt, requirements-torch.txt, and requirements-mmdet3d.txt that is not already declared in pyproject.toml (core deps or any extra: onnx, hf, puzzletron, dev-*, mlflow).
Not in pyproject.toml — new pip dependencies
| Dependency | Version in PR | License | Notes |
|---|---|---|---|
tensorrt-cu13-bindings |
==11.1.0.106 |
Proprietary (NVIDIA TensorRT SLA) | Only non-OSI license here. Already used by examples/onnx_ptq/far3d, so not new to the repo. |
plyfile |
unpinned | GPL-3.0-or-later (≤ 0.7.x) / BSD-3-Clause (≥ 1.0) | plyfile>=1.0 to guarantee BSD. Pulled in directly and transitively by lyft-dataset-sdk. |
mmcv-full |
==1.7.0 |
Apache-2.0 | OpenMMLab |
mmdet |
==2.28.2 |
Apache-2.0 | OpenMMLab |
mmsegmentation |
==0.30.0 |
Apache-2.0 | OpenMMLab |
mmdet3d |
==1.0.0rc6 |
Apache-2.0 | OpenMMLab |
nuscenes-devkit |
unpinned | Apache-2.0 (code) | |
lyft-dataset-sdk |
unpinned | Apache-2.0 (code) | Lyft Level 5 data is CC BY-NC-SA 4.0. Pulled in only because mmdet3d imports it. |
onnxsim |
unpinned | Apache-2.0 | Distinct from onnxslim in the onnx extra. |
opencv-python |
==4.5.5.64 |
MIT (wrapper) + Apache-2.0 (bundled OpenCV ≥ 4.5.0) | Wheels also bundle FFmpeg/libav (LGPL-2.1+) and other third-party libs. See the separate CVE-2023-4863 comment on this line. |
numba |
==0.57.1 |
BSD-2-Clause | Pulls llvmlite (BSD-2-Clause). |
networkx |
>=2.2 |
BSD-3-Clause | |
scikit-image |
unpinned | BSD-3-Clause | |
einops |
unpinned | MIT | |
trimesh |
unpinned | MIT | |
pycuda |
unpinned | MIT (MIT/X Consortium) | |
ipython |
<9 |
BSD-3-Clause | |
yapf |
==0.32.0 |
Apache-2.0 |
Genuinely new to the whole repository (not already in examples/onnx_ptq/far3d/requirements*.txt): lyft-dataset-sdk, networkx, numba, nuscenes-devkit, plyfile, pycuda, scikit-image, trimesh.
Already covered by pyproject.toml — no new license
numpy, setuptools, onnx, onnx-graphsurgeon, onnxruntime (onnx extra), torch and torchvision (core / dev-test). Note torch==1.13.1+cu117 and torchvision==0.14.1+cu117 are far older than the declared torch>=2.8, but that's a version-compatibility point, not a licensing one — both are BSD-3-Clause.
Non-pip dependencies this PR introduces (also outside pyproject.toml)
| Source | License |
|---|---|
nvcr.io/nvidia/pytorch:26.07-py3 base image (Dockerfile:4) |
NVIDIA Deep Learning Container License |
NVIDIA/DL4AGX clone + patch.diff (README.md:12) |
Apache-2.0 |
megvii-research/PETR clone (README.md:13) |
Apache-2.0 (Megvii copyright) |
open-mmlab/mmdetection3d v0.17.1 clone (README.md:16) |
Apache-2.0 |
Adapted source in evaluate.py (DL4AGX / OpenMMLab) and prepare_sweep_metadata.py (PETR / Megvii) |
Apache-2.0 upstream; needs the maintainer/legal ack already flagged in the review |
| nuScenes dataset (required at runtime) | CC BY-NC-SA 4.0 — non-commercial |
Actions I'd suggest
plyfile— the only copyleft exposure. Pinplyfile>=1.0(BSD-3-Clause) so no build can pull the GPLv3 line. Fix this →tensorrt-cu13-bindings— Proprietary; needs@NVIDIA/modelopt-setup-codeownerssign-off per the existing review comment (precedent exists in the FAR3D example).- Non-commercial data licenses — add a one-line note in
examples/onnx_ptq/petr/README.mdthat nuScenes is CC BY-NC-SA 4.0 and must be obtained by the user under its own terms. - Everything else is permissive (Apache-2.0 / BSD / MIT) and compatible with this repo's Apache-2.0 license.
Licenses were read from upstream project metadata and LICENSE files; PyPI/network lookups were blocked in this run, so I could not re-query package metadata live.
tensorrt-cu13-bindingsbeing classifiedProprietaryon PyPI was independently confirmed in the CodeRabbit review onrequirements.txt:23. The unpinned entries have no upper bound, so their resolved license depends on what pip picks at build time — that is whyplyfilematters.
• branchajrasane/petr
3 participants
What does this PR do?
Type of change: new example
Adds an end-to-end PETRv1 and PETRv2 ONNX PTQ example under
examples/onnx_ptq/petr. The example prepares PETRv2 sweep metadata, collects calibration inputs for the split backbone and detection-head graphs, quantizes models to INT8 or FP8, builds TensorRT engines, and evaluates accuracy on nuScenes.It also provides a reproducible Docker environment that isolates the legacy PETR PyTorch/MMCV stack from the current Model Optimizer environment. The workflow supports mixed-precision inference with an INT8 or FP8 backbone and FP16 head, plus optional head quantization.
Usage
Testing
Before your PR is "Ready for review"
Make sure you read and follow Contributor guidelines and your commits are signed (
git commit -s -S).Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded
trust_remote_code=True,torch.load(..., weights_only=False),pickle, etc.).CONTRIBUTING.md: ✅Additional Information
Summary by CodeRabbit
New Features
Documentation