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.

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

Add workaround for crash in libcudf debug build using output_indexalator in thrust::lower_bound #8432

Merged
merged 26 commits into from
Jun 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
bcb3de6
fix compile warnings
davidwendt Apr 29, 2021
bb3e427
add thrust::lower_bound workarounds
davidwendt Apr 29, 2021
a96b330
fix merge conflicts
davidwendt Apr 29, 2021
68cf55a
fix fixed-point result-type
davidwendt May 4, 2021
b777b0a
fix fixed-point hashing
davidwendt May 4, 2021
a24cbb8
fix MD5 hash to use dispatch-storage type
davidwendt May 4, 2021
9162f4f
Merge branch 'branch-0.20' into debug-dictionary-test
davidwendt May 4, 2021
29e4235
fix merge conflicts
davidwendt May 13, 2021
cd5392d
fix is_packable in concurrent_unordered_map
davidwendt May 25, 2021
b53cac3
Merge branch 'branch-21.08' into debug-dictionary-test
davidwendt May 25, 2021
d5573b7
fixed invalid access of padding bits
davidwendt May 27, 2021
bb7e82c
Merge branch 'branch-21.08' into debug-dictionary-test
davidwendt May 28, 2021
66f1625
Merge branch 'branch-21.08' into debug-dictionary-test
davidwendt May 28, 2021
882ad28
Merge branch 'branch-21.08' into debug-dictionary-test
davidwendt Jun 1, 2021
041fc0f
fix merge conflicts
davidwendt Jun 2, 2021
86b80d5
fix merge conflict error
davidwendt Jun 2, 2021
9be5dda
Merge branch 'branch-21.08' into debug-dictionary-test
davidwendt Jun 2, 2021
f0c1488
Merge branch 'branch-21.08' into debug-dictionary-test
davidwendt Jun 3, 2021
ef251e8
undo out-of-scope benchmark changes
davidwendt Jun 3, 2021
a914ebb
Merge branch 'branch-21.08' into debug-dictionary-test
davidwendt Jun 3, 2021
06ea976
fix cast statements
davidwendt Jun 3, 2021
8a8fa46
Merge branch 'branch-21.08' into debug-dictionary-test
davidwendt Jun 3, 2021
d4bb9b8
Merge branch 'branch-21.08' into debug-dictionary-test
davidwendt Jun 4, 2021
92021b3
add issue link to comment on workaround
davidwendt Jun 4, 2021
5d4abcb
Merge branch 'branch-21.08' into debug-dictionary-test
davidwendt Jun 7, 2021
70e49d3
add nvbug to workaround comment
davidwendt Jun 8, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions cpp/src/dictionary/detail/concatenate.cu
Original file line number Diff line number Diff line change
Expand Up @@ -153,19 +153,38 @@ struct dispatch_compute_indices {
}));

auto new_keys_view = column_device_view::create(new_keys, stream);

auto begin = new_keys_view->begin<Element>();
auto end = new_keys_view->end<Element>();

// create the indices output column
auto result = make_numeric_column(
all_indices.type(), all_indices.size(), mask_state::UNALLOCATED, stream, mr);
auto result_itr =
cudf::detail::indexalator_factory::make_output_iterator(result->mutable_view());
// new indices values are computed by matching the concatenated keys to the new key set

#ifdef NDEBUG
thrust::lower_bound(rmm::exec_policy(stream),
new_keys_view->begin<Element>(),
new_keys_view->end<Element>(),
begin,
end,
all_itr,
all_itr + all_indices.size(),
result_itr,
thrust::less<Element>());
#else
// There is a problem with thrust::lower_bound and the output_indexalator.
// https://github.com/NVIDIA/thrust/issues/1452; thrust team created nvbug 3322776
// This is a workaround.
thrust::transform(rmm::exec_policy(stream),
all_itr,
all_itr + all_indices.size(),
result_itr,
[begin, end] __device__(auto key) {
auto itr = thrust::lower_bound(thrust::seq, begin, end, key);
return static_cast<size_type>(thrust::distance(begin, itr));
});
#endif
return result;
}

Expand Down
37 changes: 25 additions & 12 deletions cpp/src/dictionary/set_keys.cu
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <cudf/detail/stream_compaction.hpp>
#include <cudf/detail/valid_if.cuh>
#include <cudf/dictionary/detail/encode.hpp>
#include <cudf/dictionary/detail/iterator.cuh>
#include <cudf/dictionary/dictionary_column_view.hpp>
#include <cudf/dictionary/dictionary_factories.hpp>
#include <cudf/stream_compaction.hpp>
Expand All @@ -38,6 +39,7 @@ namespace cudf {
namespace dictionary {
namespace detail {
namespace {

/**
* @brief Type-dispatch functor for remapping the old indices to new values based on the new
* key-set.
Expand All @@ -55,16 +57,11 @@ struct dispatch_compute_indices {
rmm::mr::device_memory_resource* mr)
{
auto dictionary_view = column_device_view::create(input.parent(), stream);
auto d_dictionary = *dictionary_view;
auto keys_view = column_device_view::create(input.keys(), stream);
auto dictionary_itr = thrust::make_permutation_iterator(
keys_view->begin<Element>(),
thrust::make_transform_iterator(
thrust::make_counting_iterator<size_type>(0), [d_dictionary] __device__(size_type idx) {
if (d_dictionary.is_null(idx)) return 0;
return static_cast<size_type>(d_dictionary.element<dictionary32>(idx));
}));
auto new_keys_view = column_device_view::create(new_keys, stream);
auto dictionary_itr = make_dictionary_iterator<Element>(*dictionary_view);
auto new_keys_view = column_device_view::create(new_keys, stream);

auto begin = new_keys_view->begin<Element>();
auto end = new_keys_view->end<Element>();

// create output indices column
auto result = make_numeric_column(get_indices_type_for_size(new_keys.size()),
Expand All @@ -74,14 +71,30 @@ struct dispatch_compute_indices {
mr);
auto result_itr =
cudf::detail::indexalator_factory::make_output_iterator(result->mutable_view());

#ifdef NDEBUG
thrust::lower_bound(rmm::exec_policy(stream),
new_keys_view->begin<Element>(),
new_keys_view->end<Element>(),
begin,
end,
dictionary_itr,
dictionary_itr + input.size(),
result_itr,
thrust::less<Element>());
#else
// There is a problem with thrust::lower_bound and the output_indexalator
// https://github.com/NVIDIA/thrust/issues/1452; thrust team created nvbug 3322776
// This is a workaround.
thrust::transform(rmm::exec_policy(stream),
dictionary_itr,
dictionary_itr + input.size(),
result_itr,
[begin, end] __device__(auto key) {
auto itr = thrust::lower_bound(thrust::seq, begin, end, key);
return static_cast<size_type>(thrust::distance(begin, itr));
});
#endif
result->set_null_count(0);

return result;
}

Expand Down