Adding OpenRNG Backend #2871

DhanusML · 2024-08-12T06:21:07Z

OpenRNG is an open-source Random Number Generator library developed at Arm. OpenRNG includes the interface to the random number generation part of the Vector Statistics Library (VSL) developed by Intel(R) and shipped for x86 processors as part of oneMKL. This PR adds an option to use OpenRNG as the RNG backend for oneDAL.

To build using OpenRNG backend follow these steps

.ci/scripts/build.sh --compiler gnu --optimizations sve --target onedal_c --backend-config ref  --plat lnxarm --use-openrng yes
.ci/scripts/build.sh --compiler gnu --optimizations sve --target daal --backend-config ref  --plat lnxarm --use-openrng yes

With this PR, OpenRNG backend becomes available for builds on arm machine with ref backend using gnu compiler. (For the other kinds of ref builds, support can be added in the future)

The script .ci/env/openrng.sh builds openrng at __deps/openrng. For more control over the build process, this script can be run before running the make command. While building using make, the variable RNG_BACKEND should be set to openrng for the build to use OpenRNG backend.

If openRNG is installed in a directory other than ./__deps/openrng, the environment variable OPENRNGROOT can be set before using the make command to specify the openRNG build directory.

Alexandr-Solovev · 2024-08-12T07:22:43Z

/intelci: run

DhanusML · 2024-09-03T09:22:27Z

@Alexandr-Solovev, any feedbacks on this?

Alexandr-Solovev · 2024-09-03T09:39:09Z

/intelci: run

To build using openrng backend follow these steps .ci/scripts/build.sh --compiler gnu --optimizations sve \ --target onedal_c --backend-config ref --plat lnxarm --use-openrng yes .ci/scripts/build.sh --compiler gnu --optimizations sve \ --target daal --backend-config ref --plat lnxarm --use-openrng yes With this PR, openRNG backend becomes available for builds on arm machine with ref backend using gnu compiler. (For the other kinds of ref builds, support can be added in the future) The script `.ci/env/openrng.sh` builds openrng at `__deps/openrng`. For more control over the build process, this script can be run before running the make command. While building using make, the variable `RNG_BACKEND` should be set to `openrng` for the build to use openRNG backend. If openRNG is installed in a directory other than `./__deps/openrng`, the environment variable `OPENRNGROOT` can be set before using the make command to specify the openRNG build directory. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

KulikovNikita · 2024-09-11T17:48:27Z

cpp/daal/src/externals/service_rng_openrng.h

+    {
+        services::Status s = allocSeeds(n);
+        if (s)
+        {


What about just directly reporting an error here?

I am not sure how to report an error in the constructor. I could throw an exception, but the calling functions may not have mechanisms to handle them. Also, this is exactly how the MKL backend handles it (in service_rng_mkl.h). In fact service_rng_openrng.h is a copy of service_rng_mkl.h, except for some minor changes.

KulikovNikita · 2024-09-11T17:50:39Z

cpp/daal/src/externals/service_rng_openrng.h

+        services::Status s = allocSeeds(_seedSize);
+        if (s)
+        {
+            for (size_t i = 0; i < _seedSize; i++)


It's just a matter of preferences but I would suggest to use correctly typed constants i.e. 0ul instead of an implicit conversion from 0 wherever it is posssible

ethanglaser · 2024-09-16T15:08:25Z

/intelci: run

ethanglaser · 2024-09-16T15:26:06Z

/intelci: run

ethanglaser · 2024-09-16T23:52:11Z

/intelci: run

ethanglaser · 2024-09-17T15:27:10Z

Hi @DhanusML thanks for putting this together, in order to run our internal validation the fork name must be oneDAL or daal. Would you mind changing yours from oneDAL-dhanus to just oneDAL?

DhanusML · 2024-09-17T16:32:20Z

Hi @DhanusML thanks for putting this together, in order to run our internal validation the fork name must be oneDAL or daal. Would you mind changing yours from oneDAL-dhanus to just oneDAL?

Done

ethanglaser · 2024-09-17T16:33:50Z

/intelci: run

ethanglaser · 2024-09-17T18:02:28Z

Please rebase your branch as well

ethanglaser · 2024-09-18T06:32:18Z

/intelci: run

ethanglaser · 2024-09-19T04:56:32Z

Changes look reasonable to me - but there is no validation from what I can tell. Perhaps it would make sense to add this flag to one of the ARM CI checks?

Alexandr-Solovev · 2024-09-23T07:28:06Z

cpp/daal/src/externals/service_rng_openrng.h

@@ -0,0 +1,393 @@
+/* file: service_rng_openrng.h.h */


service_rng_openrng.h

Alexandr-Solovev · 2024-09-23T07:34:13Z

cpp/daal/src/externals/service_rng_openrng.h

+        int nb     = len / 2;
+        int nn     = (int)n;
+        int * rr   = (int *)r;
+        __DAAL_VSLFN_CALL_NR_WHILE(, viRngUniform, (method, stream, nn, rr, na, nb), errcode);


why do you use DAAL macros in this file? Are you sure that it will be opened and work as expected? Can it be replaced with the direct call of openrng functions?

It can be replaced with direct calls to openrng functions. It works as expected using daal macros (from service_stat_rng_ref.h) but, I will change them to direct function calls for the sake of readability.

Alexandr-Solovev · 2024-09-23T07:36:16Z

cpp/daal/src/externals/service_rng_openrng.h

+        int nb     = len / 2;
+        int nn     = (int)n;
+        int * rr   = (int *)r;
+        __DAAL_VSLFN_CALL_NR_WHILE(, viRngUniform, (method, stream, nn, rr, na, nb), errcode);


should the method and n be casted to openrng_int_t?

Alexsandruss · 2024-09-23T10:01:05Z

.ci/scripts/build.sh

@@ -34,6 +34,7 @@ show_help() {
 --plat:The platform to build for. This is passed to the oneDAL top level Makefile
 --blas-dir:The BLAS installation directory to use to build oneDAL with in the case that the backend is given as `ref`. If the installation directory does not exist, attempts to build this from source
 --tbb-dir:The TBB installation directory to use to build oneDAL with in the case that the backend is given as `ref`. If the installation directory does not exist, attempts to build this from source
+--use-openrng:Set this to yes if openrng is to be used as RNG backend. Use this with the `ref` backend.


Suggested change

--use-openrng:Set this to yes if openrng is to be used as RNG backend. Use this with the `ref` backend.

--use-openrng:Set this to yes if openrng is to be used as RNG backend. Use this only with the `ref` backend.

Alexsandruss · 2024-09-23T10:06:14Z

makefile

+ifeq ($(RNG_BACKEND), openrng)
+	RNG_OPENRNG := yes
+endif


RNG_BACKEND variable should be one of mkl, ref and openrng with default value dependent on platform type.

Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

setting RNG_BACKEND variable based on the backend config in makefile. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

napetrov · 2024-09-27T14:57:33Z

/intelci: run

Alexandr-Solovev

LGTM

DhanusML requested review from Alexsandruss, samir-nasibli, Alexandr-Solovev, napetrov, homksei, ahuber21 and ethanglaser as code owners August 12, 2024 06:21

DhanusML added 2 commits September 5, 2024 13:41

clang-format-14 fix

f2a380d

Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

DhanusML force-pushed the dhanus/openrng-build branch from 182a11a to f2a380d Compare September 5, 2024 08:12

KulikovNikita reviewed Sep 11, 2024

View reviewed changes

ethanglaser added the enhancement label Sep 17, 2024

Merge branch 'oneapi-src:main' into dhanus/openrng-build

0cce31c

Alexandr-Solovev reviewed Sep 23, 2024

View reviewed changes

Alexsandruss requested changes Sep 23, 2024

View reviewed changes

DhanusML and others added 3 commits September 27, 2024 15:32

removed daal macros and added typecasts

f5ac14f

Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

review comments

a2ad4f3

setting RNG_BACKEND variable based on the backend config in makefile. Signed-off-by: Dhanus M Lal <Dhanus.MLal@fujitsu.com>

Merge branch 'main' into dhanus/openrng-build

b1efae0

DhanusML requested review from Alexsandruss, Alexandr-Solovev and KulikovNikita September 30, 2024 04:09

Alexandr-Solovev approved these changes Oct 1, 2024

View reviewed changes

Alexsandruss approved these changes Oct 1, 2024

View reviewed changes

Alexsandruss merged commit 7f05e43 into oneapi-src:main Oct 1, 2024
17 of 18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding OpenRNG Backend #2871

Adding OpenRNG Backend #2871

DhanusML commented Aug 12, 2024

Alexandr-Solovev commented Aug 12, 2024

DhanusML commented Sep 3, 2024

Alexandr-Solovev commented Sep 3, 2024

KulikovNikita Sep 11, 2024

DhanusML Sep 12, 2024 •

edited

Loading

KulikovNikita Sep 11, 2024

ethanglaser commented Sep 16, 2024

ethanglaser commented Sep 16, 2024

ethanglaser commented Sep 16, 2024

ethanglaser commented Sep 17, 2024

DhanusML commented Sep 17, 2024

ethanglaser commented Sep 17, 2024

ethanglaser commented Sep 17, 2024

ethanglaser commented Sep 18, 2024

ethanglaser commented Sep 19, 2024

Alexandr-Solovev Sep 23, 2024

Alexandr-Solovev Sep 23, 2024

DhanusML Sep 27, 2024

Alexandr-Solovev Sep 23, 2024

Alexsandruss Sep 23, 2024

Alexsandruss Sep 23, 2024

napetrov commented Sep 27, 2024

Alexandr-Solovev left a comment

	--use-openrng:Set this to yes if openrng is to be used as RNG backend. Use this with the `ref` backend.
	--use-openrng:Set this to yes if openrng is to be used as RNG backend. Use this only with the `ref` backend.

Adding OpenRNG Backend #2871

Adding OpenRNG Backend #2871

Conversation

DhanusML commented Aug 12, 2024

Alexandr-Solovev commented Aug 12, 2024

DhanusML commented Sep 3, 2024

Alexandr-Solovev commented Sep 3, 2024

Choose a reason for hiding this comment

DhanusML Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethanglaser commented Sep 16, 2024

ethanglaser commented Sep 16, 2024

ethanglaser commented Sep 16, 2024

ethanglaser commented Sep 17, 2024

DhanusML commented Sep 17, 2024

ethanglaser commented Sep 17, 2024

ethanglaser commented Sep 17, 2024

ethanglaser commented Sep 18, 2024

ethanglaser commented Sep 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

napetrov commented Sep 27, 2024

Alexandr-Solovev left a comment

Choose a reason for hiding this comment

DhanusML Sep 12, 2024 •

edited

Loading