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

Hardcoded random seed in unit test ReaderTest.projectColumnsMutation #12240

Closed
anlowee opened this issue Feb 3, 2025 · 0 comments
Closed

Hardcoded random seed in unit test ReaderTest.projectColumnsMutation #12240

anlowee opened this issue Feb 3, 2025 · 0 comments
Labels
bug Something isn't working triage Newly created issue that needs attention.

Comments

@anlowee
Copy link

anlowee commented Feb 3, 2025

Bug description

Description

In unit test ReaderTest.projectColumnsMutation (located at here), the random skip seed is hardcoded to 42. Since the RNG used here seems to be the std::mt199937 (as defined here), when using clang-18 compile the velox on ARM64 and AMD64 architecture, the second assertion in this unit test can only be passed on AMD64. This is very likely caused by the RNG performs differently on different architecture due to some low level reasons.

Reproduction

We compiled the velox with the following flags with clang-18

FINAL CMAKE_CXX_FLAGS= -Wno-deprecated-declarations -march=armv8-a+crc+crypto -std=c++17 -D USE_VELOX_COMMON_BASE -D HAS_UNCAUGHT_EXCEPTIONS -fsigned-char -rtlib=compiler-rt -Wall -Wextra -Wno-unused        -W
no-unused-parameter        -Wno-sign-compare        -Wno-ignored-qualifiers        -Wno-range-loop-analysis          -Wno-mismatched-tags          -Wno-vla-cxx-extension          -Wno-c++20-designator         
 -Wno-c++20-extensions          -Wno-nullability-completeness          -Wno-gnu-designator          -Wno-c99-designator

And ran the unit test with the following command:

./_build/release/velox/dwio/common/tests/velox_dwio_common_test --gtest_filter=ReaderTest.projectColumnsMutation

On ARM64, the actual vector will always be 3, 4, 7, 9 which is different from the expected vector which is 0, 1, 3, 5, 6, 8.

System information

Velox System Info v0.0.2
Commit: 9f4419e201808bd3bc9fbe85415121c15651f341
CMake Version: 3.30.3
System: Linux-6.1.112+
Arch: aarch64
C++ Compiler: /usr/bin/c++
C++ Compiler Version: 18.1.8
C Compiler: /usr/bin/cc
C Compiler Version: 18.1.8
CMake Prefix Path: /usr/local;/usr;/;/usr/local;/usr/local;/usr/X11R6;/usr/pkg;/opt

Relevant logs

Expected equality of these values:
  expected->size()
    Which is: 6
  actual->size()
    Which is: 4
[  FAILED  ] ReaderTest.projectColumnsMutation (0 ms)
[----------] 1 test from ReaderTest (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (0 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ReaderTest.projectColumnsMutation
@anlowee anlowee added bug Something isn't working triage Newly created issue that needs attention. labels Feb 3, 2025
facebook-github-bot pushed a commit that referenced this issue Feb 12, 2025
…pendent (#12298)

Summary:
**Summary**: Adjust assertion in ReaderTest.projectColumnsMutation based on platform. The assertion for the random skip is assuming it is using `__gnu_cxx::sfmt19937`, however if using `std::mt19937` then the result will be different and the unit test will fail due to this.

This PR is to fix the issue: #12240

Pull Request resolved: #12298

Reviewed By: kagamiori

Differential Revision: D69497063

Pulled By: kgpai

fbshipit-source-id: 1489f069b34ea7187018d809a16fe63b15336fe8
@anlowee anlowee closed this as completed Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage Newly created issue that needs attention.
Projects
None yet
Development

No branches or pull requests

1 participant