Document Vision v8 ONNX validation - #10
Conversation
Complexity-ML
left a comment
Complexity-ML
left a comment
There was a problem hiding this comment.
Thanks — the validation report, metadata, artifact handling, and calibrated v8 tolerances look good overall. I found one blocking regression in the default sidecar-less parity flow. Please fix it and add the regression test described inline. Also please ensure the Detector export GitHub Actions workflow runs green before merge; at review time only GitGuardian had completed.
| branch = metadata.get("branch", "auto") | ||
| if branch not in {"nms-free", "o2m"}: | ||
| raise ValueError(f"invalid export branch in {metadata_path}: {branch}") | ||
| raise ValueError(f"invalid export branch in ONNX metadata: {branch}") |
There was a problem hiding this comment.
When the ONNX sidecar is absent, sidecar_metadata() returns {}, so metadata.get("branch", "auto") yields "auto" and this raises. That breaks the previous/default --branch auto behavior even though RawDetectorExport can resolve auto, and it contradicts the CLI help saying metadata is optional. Please return "auto" when branch metadata is absent (while still rejecting an explicitly invalid value) and add assert branch_from_sidecar({}, "auto") == "auto" as a regression test.
d27fe86 to
b3c0e16
Compare
2 participants
Summary
Artifact handling
Generated ONNX binaries are not committed to the source repository.
The report documents the expected GitHub Release assets:
tr_hash_v8_o2m.onnxtr_hash_v8_o2m.jsontr_hash_v8_nms_free.onnxtr_hash_v8_nms_free.jsonThe report includes SHA-256 hashes and file sizes for verifying uploaded release artifacts.
Validation
python -m pytest tests/test_detector_export.pyscripts/check_onnx_parity.pyfull 5-test O2M parity with calibrated default tolerancescripts/check_onnx_parity.pyfull 5-test NMS-free parity with calibrated default tolerance