Skip to content

[RNG][ARMPL] Support using OpenRNG in place of ArmPL #660

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

Merged
merged 4 commits into from
May 12, 2025

Conversation

rafbiels
Copy link
Contributor

@rafbiels rafbiels commented Apr 8, 2025

Description

Introduce the possibility to use the open-source OpenRNG library instead of the one bundled in ArmPL to build the oneMath ArmPL RNG backend. It can be used in two ways:

  1. If ArmPL is not found but ArmPL RNG backend is enabled, fall back automatically to search for and use OpenRNG.
  2. User can request it explicitly with -DENABLE_ARMPL_OPENRNG=True

The benefit of the open-source version is that it also works on x86_64 CPUs, so the backend becomes more portable. It also enables users constrained to open-source projects to use the backend.

Introduce new CMake find module for OpenRNG using the standard wrappers. This means the usual system library directories will be searched and user can also specify -DOpenRNG_ROOT=<path> as with other CMake modules.

The only functional difference between OpenRNG and ArmPL is that the former does not (currently) provide any API for version checking, so the version check is skipped. This means the oneMath backend will not throw version-dependent unimplemented exceptions in case of OpenRNG even if the version used doesn't support the feature. This affects the uniform integer distributions which give faulty results in negative integer ranges in previous releases of OpenRNG/ArmPL.

Minor extra changes:

  • do not run FindARMPL.cmake version check if ArmPL is not found
  • enable RNG examples for the ArmPL backend with opencl:cpu device

Checklist

  • Do all unit tests pass locally? Attach a log.
    Tested the below configurations where aarch64 is a ubuntu:24.04/arm64 docker image emulated on x86_64 using binfmt:
    • reference - head of develop branch, aarch64, ArmPL, DPC++, POCL CPU device:
      • ut-aarch64-dpcpp-armpl-pocl.ref.gz
      • total: 162, skipped: 28, run: 134, failed: 78, passed: 56
      • I'm using POCL 5.0.2 which doesn't support some DPC++ runtime calls to OpenCL Intel extensions, hence the failures
    • same config as reference but with my changes - aarch64, ArmPL, DPC++, POCL CPU device:
      • ut-aarch64-dpcpp-armpl-pocl.log.gz
      • total: 164, skipped: 28, run: 136, failed: 79, passed: 57
      • the 2 new tests are the RNG examples which are now enabled for opencl:cpu device
    • newly supported config - aarch64, OpenRNG, DPC++, POCL CPU device:
    • newly supported config - x86_64, OpenRNG, DPC++, Intel OpenCL CPU device:
      • ut-openrng-x86_64-intelocl.log.gz
      • total: 164, skipped: 24, run: 140, failed: 4, passed: 136
      • the 4 failures are from the uniform integer distribution as explained above
  • Have you provided motivation for adding a new feature? Increased portability - see description.
  • Have you added relevant tests? No new scope for testing - existing tests cover the new backend option.

@rafbiels rafbiels requested review from a team as code owners April 8, 2025 16:16
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, thank you!

@aelizaro aelizaro added the backend A request to enable new implementation behind API label Apr 30, 2025
@aelizaro
Copy link
Contributor

@mkrainiuk or @dnhsieh-intel, could you please also take a look?

@dnhsieh-intel
Copy link
Contributor

Sorry for the very late review, and thanks for the ping!

Just want to make sure that I didn't miss something. The description and the documentation mention that the option is ENABLE_ARMPL_OPENRNG, but it looks to me that the option is ARMPL_USE_OPENRNG in the implementation.

@rafbiels
Copy link
Contributor Author

rafbiels commented May 2, 2025

Well spotted @dnhsieh-intel! I intended to rename it but forgot. Fixed now in ebf31b6.

Although my initial name ARMPL_USE_OPENRNG was more natural and accurate, I decided to go with ENABLE_ARMPL_OPENRNG to align the naming with all other options, including ENABLE_ARMPL_OMP.

@Rbiessy Rbiessy merged commit 1694bef into uxlfoundation:develop May 12, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend A request to enable new implementation behind API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants