Skip to content

Enums for bitdepths and output toggle#485

Merged
r-abishek merged 2 commits intor-abishek:ar/test_suite_mods_13from
HazarathKumarM:hk/test_suite_upgrade
Sep 10, 2025
Merged

Enums for bitdepths and output toggle#485
r-abishek merged 2 commits intor-abishek:ar/test_suite_mods_13from
HazarathKumarM:hk/test_suite_upgrade

Conversation

@HazarathKumarM
Copy link
Copy Markdown
Collaborator

No description provided.

@HazarathKumarM HazarathKumarM marked this pull request as draft September 2, 2025 04:57
@r-abishek r-abishek changed the base branch from develop to master September 3, 2025 04:26
@r-abishek r-abishek changed the base branch from master to develop September 3, 2025 04:27
@HazarathKumarM HazarathKumarM marked this pull request as ready for review September 4, 2025 03:58
@HazarathKumarM
Copy link
Copy Markdown
Collaborator Author

@r-abishek this PR ready for review

@r-abishek r-abishek requested a review from Copilot September 10, 2025 19:58
@r-abishek r-abishek added the validation Validate build and tests label Sep 10, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces enums for bitdepths and output toggle functionality to improve code readability and maintainability across the test suite. The changes replace magic numbers with descriptive enum values for bitdepths and output format toggles, making the code more self-documenting.

  • Introduces BitDepth enum with values like U8_U8, F16_F16, F32_F32, etc.
  • Adds OutputFormat and Layout enums for better type safety
  • Replaces hardcoded numeric values throughout the codebase with enum constants

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
rpp_test_suite_common.h Defines the new BitDepth enum with all supported bit depth combinations
rpp_test_suite_voxel.h Updates includes and bitdepth comparisons to use enum values
rpp_test_suite_misc.h Replaces hardcoded bitdepth values with enum constants
rpp_test_suite_image.h Updates bitdepth checks to use enum values throughout
common.py Adds Python enum classes and updates mapping functions
Various test files Updates all C++ and Python test files to use enum values instead of magic numbers
Comments suppressed due to low confidence (1)

utilities/test_suite/common.py:1

  • Same issue as above - should compare layout == Layout.PKD3 instead of just Layout.PKD3.
"""

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +449 to +453
if Layout.PKD3:
return "PKD3"
elif layout == 1:
elif Layout.PLN3:
return "PLN3"
elif layout == 2:
elif Layout.PLN1:
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The layout comparison logic is incorrect. Layout.PKD3 will always evaluate to True. Should compare with layout == Layout.PKD3.

Copilot uses AI. Check for mistakes.
Comment on lines +544 to 547
if Layout.PKD3:
result += "_PKD3_toPKD3"
elif layout == 1:
elif Layout.PLN3:
result += "_PLN3_toPLN3"
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same layout comparison issue - should use layout == Layout.PKD3 and layout == Layout.PLN3.

Copilot uses AI. Check for mistakes.
continue
for layout in range(3):
for layout in list(Layout):
dstPathTemp, logFileLayout = process_layout(layout, qaMode, case, dstPath, "hip", voxelAugmentationGroupMap, func_group_finder)
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing voxelAugmentationMap parameter in func_group_finder call - should match the updated signature.

Suggested change
dstPathTemp, logFileLayout = process_layout(layout, qaMode, case, dstPath, "hip", voxelAugmentationGroupMap, func_group_finder)
dstPathTemp, logFileLayout = process_layout(layout, qaMode, case, dstPath, "hip", voxelAugmentationGroupMap, lambda *args, **kwargs: func_group_finder(*args, voxelAugmentationMap=voxelAugmentationMap, **kwargs))

Copilot uses AI. Check for mistakes.
continue
for layout in range(3):
for layout in list(Layout):
dstPathTemp, logFileLayout = process_layout(layout, qaMode, case, dstPath, "hip", voxelAugmentationGroupMap, func_group_finder)
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing voxelAugmentationMap parameter in func_group_finder call - should match the updated signature.

Copilot uses AI. Check for mistakes.
@r-abishek r-abishek changed the base branch from develop to ar/test_suite_mods_13 September 10, 2025 20:10
@r-abishek r-abishek merged commit 487803e into r-abishek:ar/test_suite_mods_13 Sep 10, 2025
@r-abishek
Copy link
Copy Markdown
Owner

@HazarathKumarM Pls check these copilot comments and add fixes if needed

ManasaDattaT pushed a commit to ManasaDattaT/rpp that referenced this pull request Dec 19, 2025
r-abishek#485)

Bumps [rocm-docs-core[api_reference]](https://github.com/ROCm/rocm-docs-core) from 1.9.0 to 1.9.2.
- [Release notes](https://github.com/ROCm/rocm-docs-core/releases)
- [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md)
- [Commits](ROCm/rocm-docs-core@v1.9.0...v1.9.2)

---
updated-dependencies:
- dependency-name: rocm-docs-core[api_reference]
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

validation Validate build and tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants