Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RPP Rain augmentation on HOST and HIP #463

Draft
wants to merge 41 commits into
base: develop
Choose a base branch
from

Conversation

r-abishek
Copy link
Member

@r-abishek r-abishek commented Oct 23, 2024

  • Updates version to RPP 1.9.8
  • Adds a dash-cam rain augmentation on HOST and HIP
  • Adds support for U8/F16/F32/I8 bit depths with NCHW/NHWC tensor support
  • Adds relevant unit/perf tests

@r-abishek r-abishek requested a review from a team as a code owner October 23, 2024 05:45
@kiritigowda kiritigowda requested a review from rrawther October 29, 2024 20:22
@kiritigowda kiritigowda self-assigned this Oct 29, 2024
@kiritigowda kiritigowda added the enhancement New feature or request label Oct 29, 2024
@@ -478,7 +478,7 @@ def rpp_test_suite_parser_and_validator():
print_performance_tests_summary(logFile, functionalityGroupList, numRuns)

# print the results of qa tests
nonQACaseList = ['6', '8', '24', '54', '84'] # Add cases present in supportedCaseList, but without QA support
Copy link
Contributor

Choose a reason for hiding this comment

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

does this also has precision difference? why is this not added for QA?

@@ -67,7 +67,7 @@ int main(int argc, char **argv)
bool additionalParamCase = (testCase == 8 || testCase == 21 || testCase == 23 || testCase == 24 || testCase == 49 || testCase == 79);
bool kernelSizeCase = (testCase == 49);
bool dualInputCase = (testCase == 2 || testCase == 30 || testCase == 33 || testCase == 61 || testCase == 63 || testCase == 65 || testCase == 68);
bool randomOutputCase = (testCase == 6 || testCase == 8 || testCase == 84);
Copy link
Contributor

Choose a reason for hiding this comment

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

I have commented many times before. We should not use numbers for test cases. It is better to use ENUMs here

@@ -260,7 +260,7 @@ def rpp_test_suite_parser_and_validator():
subprocess.call(["make", "-j16"], cwd=".") # nosec

# List of cases supported
supportedCaseList = ['0', '1', '2', '4', '5', '6', '8', '13', '20', '21', '23', '26', '29', '30', '31', '32', '33', '34', '35', '36', '37', '38', '39', '45', '46', '49', '54', '61', '63', '65', '68', '70', '79', '80', '81', '82', '83', '84', '85', '86', '87', '88', '89', '90', '91', '92']
Copy link
Contributor

Choose a reason for hiding this comment

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

same as before. Need to change numbers to ENUMs

@@ -154,4 +154,4 @@ inline void read_bin_file(string refFile, T *binaryContent)
fseek(fp, 0, SEEK_SET);
fread(binaryContent, fsize, 1, fp);
fclose(fp);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

please revert this file as there seems to be no change

Copy link
Contributor

@rrawther rrawther left a comment

Choose a reason for hiding this comment

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

Please address review comments

CHANGELOG.md Outdated
@@ -2,9 +2,16 @@

Full documentation for RPP is available at [https://rocm.docs.amd.com/projects/rpp/en/latest](https://rocm.docs.amd.com/projects/rpp/en/latest)

## (Unreleased) RPP 1.9.8

### Changes
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### Changes
### Changed

CHANGELOG.md Outdated
## (Unreleased) RPP 1.9.4

### Changes

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this also unreleased? Because 1.9.8 is unreleased.

@kiritigowda
Copy link
Collaborator

@r-abishek - needs conflicts resolved

@kiritigowda kiritigowda marked this pull request as draft December 23, 2024 20:28
RPP Rain - Resolved Merge Conflicts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:precheckin enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants