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

[RNG] Fixed warnings in Philox tests build and test failures under Windows #1925

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

ElenaTyuleneva
Copy link
Contributor

@ElenaTyuleneva ElenaTyuleneva commented Oct 29, 2024

Fixed failed unit test details:

void counter_management_test() [Engine = philox2x32_w18] failed
void counter_management_test() [Engine = philox2x32_w30] failed
void counter_management_test() [Engine = philox4x32_w18] failed
void counter_management_test() [Engine = philox4x32_w30] failed
  • The problem appeared only under Windows, because in_mask is of type scalar_type, which is for failed casesuint_fast32_t - the bitsize of this type is implementation-defined and differs on Windows(4 bytes) and Linux(8 bytes). On Windows the higher bits of the increment were removed by (~in_mask), that is why some unit tests failed.
  • Lower bits of the shifted counter are removed by shift operation anyway, so the masking operation is not necessary there.

Fixed warning in test details:

 warning: implicit conversion from 'unsigned long long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]

  499 |             counter2[i] = std::numeric_limits<unsigned long long>::max();

Copy link
Contributor

@aelizaro aelizaro left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Contributor

@andreyfe1 andreyfe1 left a comment

Choose a reason for hiding this comment

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

LGTM. Discussed offline

@ElenaTyuleneva ElenaTyuleneva merged commit 4b45c0a into main Oct 31, 2024
22 checks passed
@ElenaTyuleneva ElenaTyuleneva deleted the dev/etyulene/windows_test_fix branch October 31, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants