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.

Sign up for GitHub

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

Jump to bottom

[SobolEngine] Update direction numbers to 21201 dims #49710

Closed
wants to merge 1 commit into from
Closed

[SobolEngine] Update direction numbers to 21201 dims #49710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 15 additions & 4 deletions aten/src/ATen/native/SobolEngineOps.cpp
View file
Open in desktop
Expand Up @@ -126,19 +126,28 @@ Tensor& _sobol_engine_initialize_state_(Tensor& sobolstate, int64_t dimension) {
TORCH_CHECK(sobolstate.dtype() == at::kLong,
"sobolstate needs to be of type ", at::kLong);

/// First row of `sobolstate` is 1
sobolstate.select(0, 0).fill_(1);

/// Use a tensor accessor for `sobolstate`
auto ss_a = sobolstate.accessor<int64_t, 2>();
for (int64_t d = 0; d < dimension; ++d) {

/// First row of `sobolstate` is all 1s
for (int64_t m = 0; m < MAXBIT; ++m) {
ss_a[0][m] = 1;
}
Comment on lines +133 to +135
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit

Suggested change
for (int64_t m = 0; m < MAXBIT; ++m) {
ss_a[0][m] = 1;
}
std::fill=n(ss_a[0], MAXBIT, 1);


/// Remaining rows of sobolstate (row 2 through dim, indexed by [1:dim])
for (int64_t d = 1; d < dimension; ++d) {
int64_t p = poly[d];
int64_t m = bit_length(p) - 1;

// First m elements of row d comes from initsobolstate
for (int64_t i = 0; i < m; ++i) {
ss_a[d][i] = initsobolstate[d][i];
}
Comment on lines 143 to 145
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit

Suggested change
for (int64_t i = 0; i < m; ++i) {
ss_a[d][i] = initsobolstate[d][i];
}
std::copy(ss_a[d], ssa_[d] + m, initsobolstate[d]);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have very little understanding of the C++ internals of torch, but this fails (your suggestion above as well). Note that initsobolstate is a C array not a tensor, not sure if that's what's the issue here. I'll leave this and the fill_n above that also fails as is for now.
...

stderr: caffe2/aten/src/ATen/native/SobolEngineOps.cpp:141:32: error: invalid operands to binary expression ('TensorAccessor<long, 2UL - 1, DefaultPtrTraits, long>' and 'int64_t' (aka 'long'))
std::copy(ss_a[d], ss_a[d] + m, initsobolstate[d]);
~~~~~~~ ^ ~
caffe2/aten/src/ATen/TensorOperators.h:110:22: note: candidate function not viable: no known conversion from 'TensorAccessor<long, 2UL - 1, DefaultPtrTraits, long>' to 'const at::Tensor' for 1st argument
AT_FORALL_BINARY_OPS(DEFINE_OPERATOR)
^
caffe2/aten/src/ATen/TensorOperators.h:82:33: note: expanded from macro 'AT_FORALL_BINARY_OPS'
#define AT_FORALL_BINARY_OPS()
^
caffe2/aten/src/ATen/TensorOperators.h:102:22: note: expanded from macro 'DEFINE_OPERATOR'
static inline Tensor operator op(const Tensor & x, Scalar y) {
^
caffe2/aten/src/ATen/TensorOperators.h:110:22: note: candidate function not viable: no known conversion from 'TensorAccessor<long, 2UL - 1, DefaultPtrTraits, long>' to 'const at::Tensor' for 1st argument
caffe2/aten/src/ATen/TensorOperators.h:82:33: note: expanded from macro 'AT_FORALL_BINARY_OPS'
#define AT_FORALL_BINARY_OPS()
^
caffe2/aten/src/ATen/TensorOperators.h:99:22: note: expanded from macro 'DEFINE_OPERATOR'
static inline Tensor operator op(const Tensor & x, const Tensor & y) {
^
caffe2/aten/src/ATen/TensorOperators.h:110:22: note: candidate function not viable: no known conversion from 'TensorAccessor<long, 2UL - 1, DefaultPtrTraits, long>' to 'c10::Scalar' for 1st argument
caffe2/aten/src/ATen/TensorOperators.h:82:33: note: expanded from macro 'AT_FORALL_BINARY_OPS'
#define AT_FORALL_BINARY_OPS(_)
^
caffe2/aten/src/ATen/TensorOperators.h:105:22: note: expanded from macro 'DEFINE_OPERATOR'
static inline Tensor operator op(Scalar x, const Tensor & y) {
^
In module 'third_party_libgcc_std' imported from caffe2/c10/core/Allocator.h:4:
third-party-buck/platform007/build/libgcc/include/c++/7.3.0/bits/stl_algobase.h:753:29: error: cannot increment value of type 'at::TensorAccessor<long, 1, DefaultPtrTraits, long>'
__niter > 0; --__niter, ++__first)
^ ~~~~~~~
third-party-buck/platform007/build/libgcc/include/c++/7.3.0/bits/stl_algobase.h:789:23: note: in instantiation of function template specialization 'std::__fill_n_a<at::TensorAccessor<long, 1, DefaultPtrTraits, long>, long, int>' requested here
return _OI(std::__fill_n_a(std::__niter_base(__first), __n, __value));
^
caffe2/aten/src/ATen/native/SobolEngineOps.cpp:133:8: note: in instantiation of function template specialization 'std::fill_n<at::TensorAccessor<long, 1, DefaultPtrTraits, long>, long, int>' requested here
std::fill_n(ss_a[0], MAXBIT, 1);
^
2 errors generated.
When running <c++ preprocess_and_compile>.
When building rule //caffe2/aten:ATen-cpu#compile-pic-SobolEngineOps.cpp.o6f3daec3,platform007-clang.


// Fill in remaining elements of v as in Section 2 (top of pg. 90) of:
// P. Bratley and B. L. Fox. Algorithm 659: Implementing sobol's
// quasirandom sequence generator. ACM Trans.
// Math. Softw., 14(1):88-100, Mar. 1988.
for (int64_t j = m; j < MAXBIT; ++j) {
int64_t newv = ss_a[d][j - m];
int64_t pow2 = 1;
Expand All @@ -152,6 +161,8 @@ Tensor& _sobol_engine_initialize_state_(Tensor& sobolstate, int64_t dimension) {
}
}

/// Multiply each column of sobolstate by power of 2:
/// sobolstate * [2^(maxbit-1), 2^(maxbit-2),..., 2, 1]
Tensor pow2s = at::pow(2, at::native::arange((MAXBIT - 1), -1, -1, sobolstate.options()));
sobolstate.mul_(pow2s);
return sobolstate;
Expand Down