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

Bugfix - RPP BitwiseAND BitwiseOR #465

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from

Conversation

r-abishek
Copy link
Member

  • Fix mismatches in output between FP16 and other bit depths due to precision differences.
  • Use rounding off operation instead of truncation.

@kiritigowda kiritigowda self-assigned this Oct 30, 2024
@kiritigowda kiritigowda added enhancement New feature or request ci:precheckin labels Oct 30, 2024
dst_f8->f1[5] = (float)((uchar)(src1_f8->f1[5]) & (uchar)(src2_f8->f1[5]));
dst_f8->f1[6] = (float)((uchar)(src1_f8->f1[6]) & (uchar)(src2_f8->f1[6]));
dst_f8->f1[7] = (float)((uchar)(src1_f8->f1[7]) & (uchar)(src2_f8->f1[7]));
dst_f8->f1[0] = (float)((uchar)(std::nearbyintf)(src1_f8->f1[0]) & (uchar)(std::nearbyintf)(src2_f8->f1[0]));
Copy link
Contributor

Choose a reason for hiding this comment

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

This is wrong as it will change the float value before doing AND. Please do reinterpret_cast instead

Copy link
Contributor

Choose a reason for hiding this comment

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

  • The logical operations (XOR, Bitwise And etc) support is provided for images only. Images can be stored in the following formats and carry the following values for the formats : U8 (0 to 255), I8 (-128 to 127), Float (0 to 1)
  • In case of float image, we rescale the images to 0 to 255 range before we perform the logical operations. While rescaling, some of the values are close to the ceil value (Eg - 224.99267578125 in case of representing 225) and to account for the same rounding operation is done here. Usage of reinterpret cast and performing the operation leads to output mismatches in this case. Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of these operators? Is this supposed to do logical or arithmetical operation?

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

@kiritigowda kiritigowda marked this pull request as draft December 23, 2024 20:27
@kiritigowda kiritigowda marked this pull request as ready for review January 8, 2025 19:24
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.

Added a comment

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.

4 participants