Closed sbrunk closed 1 year ago
That looks like an abstract class. Did you try to create an instance of a subclass instead?
It looks like the way to create a Generator is with make_generator(), so we should work on mapping that, I guess? https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/test/cpu_rng_test.cpp
Thanks for having a look and apologies, I missed your first response.
In the meantime I also realized that functions like randperm()
that take an optional generator also respect setting the global RNG state via manual_seed
which unblocked me while writing deterministic tests. :)
But being able to call make_generator()
to create Generator
instances is still a good idea IMHO.
I'd like to get back into getting this in. generator.h
is already in the presets and the make_generator
function in C++ seems to be defined here:
template<class Impl, class... Args>
Generator make_generator(Args&&... args) {
return Generator(c10::make_intrusive<Impl>(std::forward<Args>(args)...));
}
Do we need additional mapping infos because it is a template function?
Yes, we need explicit infos to instantiate the function template.
In the new version of the presets I'm working on, I'll add make_generator_cuda
and make_generator_cpu
to map at::make_generator<at::CUDAGeneratorImpl,int8_t>
and at::make_generator<at::CPUGeneratorImpl,uint64_t>
respectively.
Would that be enough ?
Yes that'd be great. In the Python impl, the Generator
constructor takes a dtype
and then decides which make_generator
to call:
if (device.type() == at::kCPU) {
self->cdata = make_generator<CPUGeneratorImpl>();
}
#ifdef USE_CUDA
else if (device.type() == at::kCUDA) {
self->cdata = make_generator<CUDAGeneratorImpl>(device.index());
}
#elif USE_MPS
else if (device.type() == at::kMPS) {
self->cdata = make_generator<MPSGeneratorImpl>();
}
So if we have the cpu and cuda variants (and mps in the future as well once we support building against mps), it should be straightforward to do it in a similar way.
Out of curiosity, is the new version of the presets you are working on a major overhaul?
There will be 4 variants:
@Namespace("at") public static native @ByVal @Name("make_generator<at::CUDAGeneratorImpl>") Generator make_generator_cuda();
@Namespace("at") public static native @ByVal @Name("make_generator<at::CUDAGeneratorImpl,int8_t>") Generator make_generator_cuda(@Const byte device_index);
@Namespace("at") public static native @ByVal @Name("make_generator<at::CPUGeneratorImpl>") Generator make_generator_cpu();
@Namespace("at") public static native @ByVal @Name("make_generator<at::CPUGeneratorImpl,uint64_t>") Generator make_generator_cpu(long seed_in);
Out of curiosity, is the new version of the presets you are working on a major overhaul?
Yes. Hopefully coming soon as a PR. Do you have any additional wishes for this version ?
Yes. Hopefully coming soon as a PR.
Looking forward to it.
Do you have any additional wishes for this version ?
None that I know of yet. Right now I'm just curious how this might affect the Scala bindings I'm working on.
With https://github.com/bytedeco/javacpp-presets/pull/1360 merged, we can now use the make_generator_cpu()
and make_generator_cuda()
variants.
Something seems odd with the mapping of Generator. Calling seed and state related methods on a
Generator
instance triggers a segfault.For instance running this snippet:
Any ideas what could be wrong here?