Jm/init - #1
Conversation
…and compile commands; update .gitignore and add tests
…e version to 0.0.0
- Introduced `chart_utils.py` for plotting grouped and simple bar charts. - Added `io_utils.py` for file I/O operations including JSON and CSV writing. - Created `summary_utils.py` for generating Markdown summaries of benchmark outputs. - Enhanced CLI argument parsing in `_vision.py` and `predict.py` to include descriptions and examples. - Updated `MBLT_Engine` in `wrapper.py` to support ONNX models alongside MXQ. - Improved error handling in `eval_ade20k.py` for invalid predictions. - Refactored tests to ensure proper functionality and coverage for new features.
…luding organization and synchronization guidelines
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2de8f2f83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Updated dataset organization functions to change default output directories from ~/.mblt_vision to ~/.mblt_model_zoo. - Enhanced validation in the calibration dataset preparation to reject non-finite values. - Improved error handling for ONNX model path resolution and added checks for explicit MXQ paths. - Added tests for validating calibration datasets and ensuring proper handling of difficult regions in DOTAv1 evaluation. - Refactored plotting functions to maintain raw output coordinates and ensure correct color format conversion. - Introduced new tests for engine initialization to verify proper disposal of backends on setup failures.
|
@mobilint-review Review this PR. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex review
Requested by @parkjinman98.
Verdict
Found six significant issues, including two filesystem-safety defects, incorrect semantic benchmark geometry, and an unvalidated PyPI release path.
Suggested next steps
- Fix the two path-containment issues and add malicious XML/symlink regression tests.
- Restore semantic outputs using loader metadata and add a non-square native-resolution metric test.
- Gate publication on wheel-content, install/import, and supported-Python checks.
- Make empty postprocess tensors device-preserving and defer cache creation until first use.
Trigger: issue_comment
Source: #1 (comment)
|
@mobilint-review Review this PR. You may ignore the semantic prediction restoration. It is not applied in purpose to keep compatibility with ultralytics api. Find other bugs or things to improve. |
There was a problem hiding this comment.
Codex review
Requested by @parkjinman98.
Verdict
Found three actionable issues outside the intentionally omitted semantic prediction restoration: the release workflow can publish an untested wheel, cache resolution can delete an existing user file, and segmentation postprocessing can silently truncate batches.
Suggested next steps
- Add clean-wheel installation, import smoke, and offline tests to the build job before artifact upload.
- Replace the fixed cache writability probe with a uniquely created temporary file.
- Add a regression test proving mismatched detection/prototype batch sizes fail loudly.
Trigger: issue_comment
Source: #1 (comment)
…l API; enhance cache directory handling
|
@mobilint-review Review this PR. You may ignore the semantic prediction restoration. It is not applied in purpose to keep compatibility with ultralytics api. Find other bugs or things to improve. |
There was a problem hiding this comment.
Codex review
Requested by @parkjinman98.
Verdict
I excluded semantic-prediction restoration as requested. The remaining review found two release-significant correctness/packaging issues and two bounded preprocessing/cache defects.
Suggested next steps
- Add regression coverage proving missing explicit ONNX paths fail without attempting a download.
- Make hardware runtime dependencies lazy or optional so base and ONNX-only imports satisfy the package contract.
- Use a reusable or cleaned fallback cache and cover the unwritable-home case.
- Test every accepted resize mode for PIL, NumPy, and tensor inputs.
Trigger: issue_comment
Source: #1 (comment)
|
@mobilint-review Review this PR. You may ignore the semantic prediction restoration. It is not applied in purpose to keep compatibility with ultralytics api. Find other bugs or things to improve. |
There was a problem hiding this comment.
Codex review
Requested by @parkjinman98.
Verdict
I excluded semantic prediction restoration as requested. The remaining review found two serious correctness issues affecting ONNX routing and COCO pose metrics, plus two bounded reliability gaps.
Suggested next steps
- Make all framework-specific path aliases participate in framework inference and conflict validation.
- Remove the pose image-ID filter and add a regression test containing an image without positive keypoint annotations.
- Validate split-head cardinality and require the offline test suite to pass before publishing artifacts.
Trigger: issue_comment
Source: #1 (comment)
…r split head counts and enhance tests
|
@mobilint-review Review this PR. You may ignore the semantic prediction restoration. It is not applied in purpose to keep compatibility with ultralytics api. Also, before publish, we do sufficient tests, so publish.yml will not include extra tests. Find other bugs or things to improve. |
There was a problem hiding this comment.
Codex review
Requested by @parkjinman98.
Verdict
Found one actionable benchmark defect outside the two requested exclusions. Semantic prediction restoration and additional publish-workflow tests were not flagged.
Suggested next steps
- Include
onnx_pathin the benchmark runner's framework resolution. - Add runner-level coverage showing that
--onnx-pathselects only the neutralonnxmode, including when--core-mode allis supplied.
Trigger: issue_comment
Source: #1 (comment)
|
@codex review Commit c150f2f resolves the latest actionable feedback: direct COCO taxonomy validation, direct WiderFace metadata validation, staged COCO image-geometry validation, registry-aligned ADE20K cache defaults, and rejection of all-ignore semantic targets in staging and direct loaders. Please focus on validation correctness for malformed local artifacts, dataset/cache integrity, and direct API/CLI behavior. In particular, review taxonomy and evaluation boundaries consistently across the repository rather than proposing isolated, conflicting checks. Please ignore semantic-logit restoration to original image geometry in ADE20K/Cityscapes evaluation: it is intentionally deferred to preserve Ultralytics-compatible validation behavior and is documented with a TODO. The ImageNet and DOTAv1 canonical-identity provenance findings remain intentionally open: this package needs an authoritative pinned manifest or archive digest rather than one derived from mutable local inputs; see the prior PR comments for the constraint. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c150f2f281
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Commit a58723d resolves the latest actionable feedback: direct COCO annotation-category validation, direct WiderFace image/metadata identity validation, malformed WiderFace difficulty-shape rejection, metric-valid NYU staging, strict DOTAv1 export cardinality, and cache-aware implicit compilation dataset paths. Please focus on validation correctness for malformed local artifacts, dataset/cache integrity, and direct API/CLI behavior. In particular, review taxonomy and evaluation boundaries consistently across the repository rather than proposing isolated, conflicting checks. Please ignore semantic-logit restoration to original image geometry in ADE20K/Cityscapes evaluation: it is intentionally deferred to preserve Ultralytics-compatible validation behavior and is documented with a TODO. The ImageNet and DOTAv1 canonical-identity provenance findings remain intentionally open: this package needs an authoritative pinned manifest or archive digest rather than one derived from mutable local inputs; see the prior PR comments for the constraint. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a58723d2d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Commit 2ce00fb resolves the latest actionable feedback: shared COCO task-payload validation for direct evaluation and acceptance of valid empty WiderFace difficulty lists while rejecting malformed row vectors. Please focus on validation correctness for malformed local artifacts, dataset/cache integrity, and direct API/CLI behavior. In particular, review taxonomy and evaluation boundaries consistently across the repository rather than proposing isolated, conflicting checks. Please ignore semantic-logit restoration to original image geometry in ADE20K/Cityscapes evaluation: it is intentionally deferred to preserve Ultralytics-compatible validation behavior and is documented with a TODO. The ImageNet and DOTAv1 canonical-identity provenance findings remain intentionally open: this package needs an authoritative pinned manifest or archive digest rather than one derived from mutable local inputs; see the prior PR comments for the constraint. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ce00fb25c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else None | ||
| ) | ||
| if not _coco_task_annotations_valid( | ||
| list(annotations.values()), |
There was a problem hiding this comment.
When a direct annotation JSON contains duplicate annotation IDs, the COCO constructor has already indexed it into coco.anns, where later records overwrite earlier ones; passing only annotations.values() here therefore gives the shared duplicate-ID check an already-unique list. This silently drops ground truths and can produce plausible or inflated AP, so validate the original annotation list before this lossy indexing, along with the raw image and category ID tables.
AGENTS.md reference: AGENTS.md:L88-L89
Useful? React with 👍 / 👎.
| for table in difficulties: | ||
| try: | ||
| event_indices = table[event_index][0] | ||
| except (IndexError, TypeError): | ||
| return False | ||
| if len(event_indices) != len(image_names): |
There was a problem hiding this comment.
When the Easy, Medium, and Hard files are swapped or otherwise contain individually valid but non-nested index lists, this loop validates each table independently and accepts the artifact. WiderFace difficulty ground truths are cumulative (easy ⊆ medium ⊆ hard); violating that invariant changes the denominators and eligible faces for each reported AP while still producing plausible scores in both direct and CLI evaluation, so compare the three per-image index sets before declaring the metadata valid.
AGENTS.md reference: AGENTS.md:L88-L89
Useful? React with 👍 / 👎.
|
@codex review Please review the current PR head, 2ce00fb, for new correctness regressions. The six findings just resolved were generated against the older fe2ecf7 snapshot and are already implemented in ancestor commit bf92255 (DOTA stem uniqueness, configured model-path suffix validation, direct DOTA duplicate-ID rejection, softmax probability validation, private preferred-cache validation, and calibration geometry checks). Please focus on validation correctness for malformed local artifacts, dataset/cache integrity, and direct API/CLI behavior. Review taxonomy and evaluation boundaries consistently across the repository rather than proposing isolated or conflicting checks. Please ignore semantic-logit restoration to original image geometry in ADE20K/Cityscapes evaluation: it is intentionally deferred to preserve Ultralytics-compatible validation behavior and is documented with a TODO. The ImageNet and DOTAv1 canonical-identity provenance findings remain intentionally open: this package needs an authoritative pinned manifest or archive digest rather than one derived from mutable local inputs; see the prior PR comments for the constraint. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ce00fb25c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Commit 065bc47 resolves the latest actionable feedback: it validates ground-truth foreground geometry against decoded images for COCO instance masks, WiderFace boxes, and DOTAv1 polygons; rejects unequal WiderFace box/score outputs; and rejects DOTAv1 orphan labels or evaluations without positive ground truth. The DOTAv1 staging path now enforces the same polygon geometry rule. Please focus on validation correctness for malformed local artifacts, dataset/cache integrity, and direct API/CLI behavior. Review taxonomy and evaluation boundaries consistently across the repository rather than proposing isolated or conflicting checks. Please ignore semantic-logit restoration to original image geometry in ADE20K/Cityscapes evaluation: it is intentionally deferred to preserve Ultralytics-compatible validation behavior and is documented with a TODO. The ImageNet and DOTAv1 canonical-identity provenance findings remain intentionally open: this package needs an authoritative pinned manifest or archive digest rather than one derived from mutable local inputs; see the prior PR comments for the constraint. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 065bc47dae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Commit ee9150b resolves the latest actionable feedback: it consistently scales normalized DOTA labels, validates rasterized COCO polygon foreground and visible-keypoint bounds, fully decodes WiderFace images during readiness, and requires positive targets in staged DOTA data and each WiderFace difficulty split. Please focus on validation correctness for malformed local artifacts, dataset/cache integrity, and direct API/CLI behavior. Review taxonomy and evaluation boundaries consistently across the repository rather than proposing isolated or conflicting checks. Please ignore semantic-logit restoration to original image geometry in ADE20K/Cityscapes evaluation: it is intentionally deferred to preserve Ultralytics-compatible validation behavior and is documented with a TODO. The ImageNet and DOTAv1 canonical-identity provenance findings remain intentionally open: this package needs an authoritative pinned manifest or archive digest rather than one derived from mutable local inputs; see the prior PR comments for the constraint. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee9150b1c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Commit a91d56d resolves the latest actionable feedback: it bounds pose area to annotation geometry, decodes and shape-checks cached COCO images, aggregates WiderFace difficulty positives across the whole split, rejects duplicate WiderFace boxes, and validates distinct consistently ordered DOTAv1 quadrilateral vertices in staging and direct evaluation. Please focus on validation correctness for malformed local artifacts, dataset/cache integrity, and direct API/CLI behavior. Review taxonomy and evaluation boundaries consistently across the repository rather than proposing isolated or conflicting checks. Please ignore semantic-logit restoration to original image geometry in ADE20K/Cityscapes evaluation: it is intentionally deferred to preserve Ultralytics-compatible validation behavior and is documented with a TODO. The ImageNet and DOTAv1 canonical-identity provenance findings remain intentionally open: this package needs an authoritative pinned manifest or archive digest rather than one derived from mutable local inputs; see the prior PR comments for the constraint. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a91d56dabe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex Re-review requested for commit 0fd5226. Please focus on the new cache-readiness validation (decoding paired dense images/targets, geometry and taxonomy checks), DOTA duplicate-target rejection, and COCO/WiderFace annotation safety. The offline suite passes: 564 passed, 16 deselected. Please do not reopen the intentionally deferred semantic original-geometry restoration: it remains a documented compatibility TODO until Ultralytics adopts the same behavior. Report concrete new correctness or regression issues; avoid revisiting previously settled taxonomy-policy direction without a reproducible flaw. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fd5226975
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| f"Unsupported DOTAv1 difficulty flag {parts[9]!r} at " | ||
| f"{label_path}:{line_number}." | ||
| ) | ||
| target_key = (cls, tuple(coords.flatten().tolist())) |
There was a problem hiding this comment.
When two same-class rows describe the same quadrilateral with a cyclically shifted starting vertex or reversed winding, both pass the vertex-order checks but produce different ordered tuples here, so the duplicate remains in the AP denominator even though both rows convert to the same OBB. Fresh evidence in this revision is that all newly added duplicate keys preserve the input vertex sequence; canonicalize valid quadrilaterals before constructing the key in both organization and direct evaluation paths.
AGENTS.md reference: AGENTS.md:L131-L136
Useful? React with 👍 / 👎.
| or depth.shape != image_shape | ||
| or not np.isfinite(depth).all() | ||
| or bool((depth < 0).any()) | ||
| or not bool(((depth >= 0.001) & (depth <= 100.0)).any()) |
There was a problem hiding this comment.
When a depth map's only candidate pixels equal exactly 0.001 or 100.0, this inclusive predicate declares the sample ready, but both _validate_staged_nyu_depth() and NYUDepthMetricAccumulator.update() use the strict (0.001, 100.0) interval and evaluation raises that the sample has no valid pixels. Use strict comparisons here so readiness repairs the cache instead of allowing a predictable post-inference failure.
AGENTS.md reference: AGENTS.md:L131-L136
Useful? React with 👍 / 👎.
| not np.issubdtype(depth.dtype, np.number) | ||
| or np.issubdtype(depth.dtype, np.complexfloating) | ||
| or depth.ndim != 2 | ||
| or depth.shape != image_shape | ||
| or not np.isfinite(depth).all() |
There was a problem hiding this comment.
When a real numeric target uses a wider dtype and contains a finite value outside float32 range alongside an ordinary valid depth, the raw-array finiteness check passes and readiness returns true. The dataset and staged validator convert the complete target to float32 first, turning that value into infinity and rejecting it, so this cache still fails during evaluation; perform the same float32 conversion before validating finiteness, geometry, and metric values.
AGENTS.md reference: AGENTS.md:L131-L136
Useful? React with 👍 / 👎.
| and height > 0 | ||
| and isinstance(width, int) | ||
| and not isinstance(width, bool) | ||
| and width > 0 | ||
| else None |
There was a problem hiding this comment.
When direct COCO metadata uses float dimensions that are numerically equal to the decoded image dimensions, such as 10.0, this records the shape as None, causing the shared validator to skip labeled-keypoint and polygon image-bound checks. CustomCOCODataset later considers (10.0, 10.0) == (10, 10) true, so evaluation proceeds and malformed ground truth can alter AP; fresh evidence after the labeled-keypoint fix is that an invalid dimension type silently disables that new predicate, so reject the image record instead of storing None.
AGENTS.md reference: AGENTS.md:L131-L136
Useful? React with 👍 / 👎.
|
@codex Note on the two remaining canonical-identity threads:
This repository does not currently ship either source of truth. I have intentionally not guessed or synthesized a mapping, because that would create a misleading readiness contract. The existing structural/identity validation remains in place; please treat these two requests as deferred until the corresponding authoritative manifests are added. |
|
To use Codex here, create an environment for this repo. |
|
@codex Re-review requested for commit c1ff889. This update hardens direct and cached validation: raw COCO record identity/geometry checks, depth and Cityscapes cache validity, malformed WiderFace metadata handling, and winding-independent DOTA duplicate detection. The offline suite passes: 573 passed, 16 deselected. Please focus on concrete regressions in those paths. Keep the intentionally deferred semantic original-geometry restoration out of scope. The two canonical ImageNet/DOTAv1 identity-manifest requests remain open pending authoritative pinned manifests; please do not request guessed mappings. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
No description provided.