Detect stale outputs in Laplacian pyramid/reconstruct tests (#37)#40
Open
kiritigowda wants to merge 2 commits into
Open
Detect stale outputs in Laplacian pyramid/reconstruct tests (#37)#40kiritigowda wants to merge 2 commits into
kiritigowda wants to merge 2 commits into
Conversation
The LaplacianPyramid and LaplacianReconstruct graph-processing tests executed the graph only once, so an implementation that returned stale outputs on a re-run with new input would still pass. Modify the input (bitwise-NOT), rebuild the reference from the modified input for a fair comparison, and re-run the OpenVX graph before comparing. The allocated CT_Image is garbage-collected at test teardown; the derived vx_image is released explicitly. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Updates Laplacian pyramid/reconstruct graph-processing tests to detect implementations that incorrectly return stale outputs when re-processing with changed input.
Changes:
- Adds a second run of each test using a bitwise-NOT modified copy of the input image.
- Rebuilds the reference results from the modified input before re-comparing outputs.
- Releases the temporary
vx_imagecreated from the modifiedCT_Image.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot noted the first version rebuilt a fresh graph on the second run (own_laplacian_*_openvx creates/processes/releases a graph per call), so it did not actually exercise vxProcessGraph twice on one graph — the exact stale-output scenario issue #37 targets. It also flagged the data.y access as assuming single-plane U8. - Add own_laplacian_pyramid_openvx_rerun and own_laplacian_reconstruct_openvx_rerun that keep a single graph, process it, update the input in place via ct_image_copyto_vx_image (same handle; reconstruct also refills the pyramid + lowest-res inputs from the modified source), then process the same graph again. - Compute the reference from the modified input after the re-run so the comparison stays fair. - Assert input->format == VX_DF_IMAGE_U8 before the data.y pixel modification (both Laplacian generators already produce U8). Co-authored-by: Cursor <cursoragent@cursor.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
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.
Summary
Fixes #37. The
LaplacianPyramid.testGraphProcessingandLaplacianReconstruct.testGraphProcessingtests calledvxProcessGraphonly once. An implementation that returns stale outputs on a re-run with new input would pass these tests undetected.Changes
In both graph-processing tests, after the initial run:
vx_imageand rebuild the reference from the modified input (so the comparison stays fair for implementations that correctly recompute)Notes specific to this CTS codebase:
CT_Imageis registered for garbage collection at allocation, so it is freed at test teardown (this repo has noct_free_image).vx_imageis released explicitly withvxReleaseImage.This ports the
#37fix from #36, applied in isolation and adapted to the current sources.Test plan
--filter=LaplacianPyramid.*:LaplacianReconstruct.*and confirm passMade with Cursor