Implementing cunumeric.random.BitGenerator #254

fduguet-nv · 2022-04-01T19:12:25Z

No description provided.

…o fduguet

.pre-commit-config.yaml

cunumeric/deferred.py

src/cunumeric/random/bitgenerator_curand.inl

magnatelee · 2022-07-13T15:04:46Z

src/cunumeric/cunumeric_c.h

+
+// Match these to BitGeneratorDistribution in config.py
+enum CuNumericBitGeneratorDistribution {
+  CUNUMERIC_BITGENDIST_INTEGERS_32    = 1,


are we expecting to add more distributions? if so, let's leave out enum values except the first one, otherwise you would need to renumber them every time you add a new distribution.

Yes, same point as above. Can we push that to next PR ?

magnatelee · 2022-07-13T15:19:44Z

cunumeric/random/bitgenerator.py

+        )
+        return res
+
+    def integers(


any reason you added distribution methods to BitGenerator? it looks like the BitGenerator in both NumPy and CuPy is a simple opaque handle with only a couple of methods. I guess we could keep these distribution methods only in Generator.

Yes, we could. I found it easier that way: the two options I saw were:

this way

having Generator use internal fields from BitGenerator (self.seed, self.flags, self.generatorType), and make Generator aware of underlying implementation of BitGenerator.
I favored the first one. Should you prefer otherwise (option 2 or another option), please advise.

magnatelee · 2022-07-13T15:20:25Z

cunumeric/random/generator.py

+    def __init__(self, bit_generator):
+        self.bit_generator = bit_generator
+
+    def integers(


(I moved my comment to a more appropriate line) can this replace the legacy random.randint implementation?

Yes. That it the target. It seems that randint is used by other parts. I wanted to split the sources of potential issues: cunumeric.random errors vs other errors from tests using cunumeric.random. Though, since this is the "default generator" we could use any such as the one you proposed. Both are fine.

magnatelee · 2022-07-13T15:21:03Z

cunumeric/random/generator.py

+from cunumeric.random.bitgenerator import XORWOW, BitGenerator
+
+
+class Generator:


any reason you left out uniform? I feel it's straightforward to add it given that you already have the implementation.

missing test and last API link - fixed in commit 7364d28

magnatelee · 2022-07-13T15:32:39Z

src/cunumeric/random/bitgenerator_curand.inl

+                  std::vector<legate::Store>& args)
+  {
+    generator_map_t& genmap = get_generator_map();
+    // printtid((int)op);


please remove this.

magnatelee · 2022-07-13T15:33:55Z

cunumeric/runtime.py

+        self, generatorType, seed, flags, forceCreate=False
+    ):
+        self.current_random_bitgenid = self.current_random_bitgenid + 1
+        if forceCreate:


do you have any test that exercises this path? otherwise this code path can be bitrotted

test_force_build

magnatelee · 2022-07-13T15:35:06Z

src/cunumeric.mk

 							 cunumeric/mapper.cc

-GEN_CPU_SRC += cunumeric/cephes/chbevl.cc \
+GEN_CPU_SRC += cunumeric/cephes/chbevl.cc                             \


please revert this bogus change

moved to random/random.mk to avoid such issues (and merge conflicts)

magnatelee · 2022-07-13T15:35:27Z

src/cunumeric.mk

 							 cunumeric/cephes/i0.cc

 ifeq ($(strip $(USE_OPENMP)),1)
-GEN_CPU_SRC += cunumeric/ternary/where_omp.cc          \
+GEN_CPU_SRC += cunumeric/ternary/where_omp.cc                        \


same here. (I bet this is from the IDE you're using)

In the code, we don't use tabs; here we do. What is the tab <> #spaces equivalent I should use ?

moved to random/random.mk to avoid such issues (and merge conflicts)

magnatelee · 2022-07-13T15:40:10Z

src/cunumeric/random/randutil/generator_host.cc

+#include "generator.h"
+#include "generator_create.inl"
+
+extern "C" curandStatus_t CURANDAPI randutilCreateGeneratorHost(randutilGenerator_t* generator,


can we remove CURANDAPI? I don't think we're defining a cuRAND API here (same applies to all the other functions)

magnatelee · 2022-07-13T15:52:21Z

left several questions and comments. otherwise looks good. (there are some refactoring changes I'd do but I'll do them in a follow-on PR, as this was sitting too long.) once you address the remaining comments and @marcinz confirms the CPU only build, we can merge this PR.

marcinz · 2022-07-14T01:43:02Z

src/cunumeric/random/randutil/randutil_curand.h

+#include <curand_mrg32k3a.h>
+#undef __device__
+
+#define __CUDACC_RTC__


@fduguet-nv Did you mean to #undef this after line 43? I've spent quite a lot of time trying to compile CPU package, and I finally figured out that this is the line that causes the following error in my compilation:

In file included from /home/mzalewski/.pyenv/versions/miniforge3/conda-bld/cunumeric_1657756345722/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include/cuda/std/cstddef:32, from /home/mzalewski/.pyenv/versions/miniforge3/conda-bld/cunumeric_1657756345722/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include/cuda/std/type_traits:17, from /home/mzalewski/.pyenv/versions/miniforge3/conda-bld/cunumeric_1657756345722/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include/curand_kernel.h:72, from cunumeric/random/randutil/randutil_curand.h:95, from cunumeric/random/randutil/generator.h:22, from cunumeric/random/randutil/generator_host_straightforward.cc:17: /home/mzalewski/.pyenv/versions/miniforge3/conda-bld/cunumeric_1657756345722/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include/cuda/std/detail/libcxx/include/cstddef:58:9: error: 'ptrdiff_t' has not been declared in '::' 58 | using ::ptrdiff_t; | ^~~~~~~~~ compilation terminated due to -Wfatal-errors.

If I "fix" this, then I get a different error:

In file included from /home/mzalewski/.pyenv/versions/miniforge3/conda-bld/cunumeric_1657762144072/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include/driver_types.h:61, from /home/mzalewski/.pyenv/versions/miniforge3/conda-bld/cunumeric_1657762144072/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include/builtin_types.h:59, from /home/mzalewski/.pyenv/versions/miniforge3/conda-bld/cunumeric_1657762144072/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include/cuda_runtime.h:91, from /home/mzalewski/.pyenv/versions/miniforge3/conda-bld/cunumeric_1657762144072/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include/curand.h:59, from /home/mzalewski/.pyenv/versions/miniforge3/conda-bld/cunumeric_1657762144072/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include/curand_kernel.h:99, from cunumeric/random/randutil/randutil_curand.h:96, from cunumeric/random/randutil/generator.h:22, from cunumeric/random/randutil/generator_host_straightforward.cc:17: /home/mzalewski/.pyenv/versions/miniforge3/conda-bld/cunumeric_1657762144072/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include/vector_types.h:185:30: error: redefinition of 'struct uint2' 185 | __cuda_builtin_vector_align8(uint2, unsigned int x; unsigned int y;); | ^~~~~ /home/mzalewski/.pyenv/versions/miniforge3/conda-bld/cunumeric_1657762144072/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_p/include/vector_types.h:93:40: note: in definition of macro '__cuda_builtin_vector_align8' 93 | struct __device_builtin__ __align__(8) tag \ | ^~~ compilation terminated due to -Wfatal-errors.

* Implementing BitGenerator - GPU part - creation * Plugging destroy * skip ahead for GPU BitGenerator random_raw * Refectoring test * Piping done for BitGenerator.random_raw * LGT-260 -- implemented random_raw for CUDA * Implementing BitGenerator - GPU part - creation * Plugging destroy * skip ahead for GPU BitGenerator random_raw * Refectoring test * Piping done for BitGenerator.random_raw * LGT-260 -- implemented random_raw for CUDA * Updating pre-commit * Adding Multi-GPU support * Refactoring * refactoring * CPU support - issues remain on multi-cpu mapper * Attempts to support multi-dimension * Finalizing multi-cpu implementation of BitGenerator * Repro behavior of BitGenerator * Adding random in tests * Fixes from code review - part 1 * use of logger * Enums improvement * More on code review * code review * Removing task in python destructor * Using Shape * Lazy destroy also at destroy * Using create_buffer for temporary buffer creation * Fixing bugs following merge * Adding alternate lazy init implementation * Adding force create and force destroy * Removing constraint in mapper * Updating test * Fixing Eager code branch * Updating license information and adding integer generator * PR review * Removing reference to removed omp file * Changes from PR comments * Adding generator.random implementation * Adding generator.lognormal implementation * Adding generator.normal implementation * Adding generator.poisson implementation * Refectoring randutil to allow host-only compilation without CUDA enabled * Moving distributions to HOST only capable * Minor refactoring to split compilation of further distributions * Fixing openmp test failure * Adding random.generator.exponential * Adding random.generator.gumbel * Splitting distributions in several files * Actually applying the WAR * Adding random.generator.laplace * Adding random.generator.logistic * Adding random.generator.pareto * Adding random.generator.power * Adding random.generator.rayleigh * Adding random.generator.standard_cauchy * Adding random.generatortriangular * Adding random.generator.weibul * Adding random.generator.bytes * PR review * PR review * PR review * PR review * fixing glitch * More work on PR * Fixing CURANDAPI * Moving logger symbol from .cu to .cc * removing destroy

fduguet-nv and others added 24 commits March 21, 2022 19:41

Implementing BitGenerator - GPU part - creation

d8537be

Plugging destroy

6f8437d

skip ahead for GPU BitGenerator random_raw

e997e8d

Refectoring test

e805210

Piping done for BitGenerator.random_raw

536c871

LGT-260 -- implemented random_raw for CUDA

14304e8

Implementing BitGenerator - GPU part - creation

75fea76

Plugging destroy

c436346

skip ahead for GPU BitGenerator random_raw

bec87f7

Refectoring test

c3b1000

Piping done for BitGenerator.random_raw

7133cf8

LGT-260 -- implemented random_raw for CUDA

55c2d4c

Updating pre-commit

fdb284b

Updating pre-commit

80e2c66

Adding Multi-GPU support

3ffbd6c

Merge branch 'nv-legate:branch-22.05' into fduguet

4b7ee28

Refactoring

c2e71e0

refactoring

601dbbb

CPU support - issues remain on multi-cpu mapper

006cd04

Merge branch 'fduguet' of https://github.com/fduguet-nv/cunumeric int…

f1ae1be

…o fduguet

Attempts to support multi-dimension

e8be760

Finalizing multi-cpu implementation of BitGenerator

a6944a9

merging

3bed169

Repro behavior of BitGenerator

4ad2adb

fduguet-nv marked this pull request as ready for review April 1, 2022 19:13

fduguet-nv and others added 3 commits April 8, 2022 20:44

Merge branch 'nv-legate:branch-22.05' into fduguet

65d38c4

Adding random in tests

5f283f2

Merge branch 'fduguet' of https://github.com/fduguet-nv/cunumeric int…

7b8702e

…o fduguet

magnatelee reviewed Apr 9, 2022

View reviewed changes

Fixes from code review - part 1

b7d103a

magnatelee reviewed Jul 13, 2022

View reviewed changes

marcinz reviewed Jul 14, 2022

View reviewed changes

fduguet-nv added 14 commits July 15, 2022 11:49

PR review

90135c9

PR review

7364d28

PR review

cacee28

PR review

37df124

merge

f530847

fixing glitch

2450038

Merge remote-tracking branch 'origin/branch-22.07' into fduguet

ee2c203

More work on PR

1a7ff8c

Merge remote-tracking branch 'origin/branch-22.07' into fduguet

a40a94f

Fixing CURANDAPI

c3c7d7e

Moving logger symbol from .cu to .cc

6dfb548

removing destroy

185c354

Merge

997e848

Merge remote-tracking branch 'origin/branch-22.07' into fduguet

68b3a01

fduguet-nv merged commit bf81d95 into nv-legate:branch-22.07 Jul 26, 2022

bryevdv mentioned this pull request Jul 26, 2022

Fixup typing regressions #471

Merged

ipdemes pushed a commit to ipdemes/cunumeric that referenced this pull request Aug 4, 2022

Remove some unused code in the driver (nv-legate#254)

6ef7ca9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing cunumeric.random.BitGenerator #254

Implementing cunumeric.random.BitGenerator #254

fduguet-nv commented Apr 1, 2022

magnatelee Jul 13, 2022

fduguet-nv Jul 15, 2022

magnatelee Jul 13, 2022

fduguet-nv Jul 15, 2022

magnatelee Jul 13, 2022

fduguet-nv Jul 15, 2022

magnatelee Jul 13, 2022

fduguet-nv Jul 15, 2022

magnatelee Jul 13, 2022

fduguet-nv Jul 15, 2022

magnatelee Jul 13, 2022

fduguet-nv Jul 15, 2022

magnatelee Jul 13, 2022

fduguet-nv Jul 15, 2022

fduguet-nv Jul 15, 2022 •

edited

Loading

magnatelee Jul 13, 2022

fduguet-nv Jul 15, 2022

fduguet-nv Jul 15, 2022

magnatelee Jul 13, 2022

fduguet-nv Jul 15, 2022

magnatelee commented Jul 13, 2022

marcinz Jul 14, 2022

marcinz Jul 14, 2022

		from cunumeric.random.bitgenerator import XORWOW, BitGenerator


		class Generator:

Implementing cunumeric.random.BitGenerator #254

Implementing cunumeric.random.BitGenerator #254

Conversation

fduguet-nv commented Apr 1, 2022

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fduguet-nv Jul 15, 2022 • edited Loading

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

magnatelee commented Jul 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fduguet-nv Jul 15, 2022 •

edited

Loading