Skip to content

Fix invalid-device-fn error in cudf::strings::replace_re with multiple regex's - #7336

Merged
rapids-bot[bot] merged 3 commits into
NVIDIA:branch-0.19from
davidwendt:build-gcc9
Feb 10, 2021
Merged

Fix invalid-device-fn error in cudf::strings::replace_re with multiple regex's#7336
rapids-bot[bot] merged 3 commits into
NVIDIA:branch-0.19from
davidwendt:build-gcc9

Conversation

@davidwendt

Copy link
Copy Markdown
Contributor

Found errors when testing libcudf built with gcc-9 and nvcc 11.1.

STRINGS_TEST : StringsReplaceTests.ReplaceMultiRegexTest
MERGE_TEST : MergeStringTest/8.Merge2StringKeyColumns (with BOOL8 column types only)
PARTITIONING_TEST : RoundRobinTest/8.RoundRobin (tests with BOOL8 column types only)

The strings test cause an invalid-device-function error. The code for cudf::strings::replace_re that accepts multiple regex's was fixed to better manage the device memory holding the regex structures. The error occurred when cleaning up the temporary memory.

The other two tests failed because the iterator used for generating the expected data (in CPU code) was producing the wrong results. This may be a gcc optimization bug since adding a printf cleared the error. I was able to recode the iterator's functor so it would succeed on both gcc9 and gcc7.

@davidwendt davidwendt added bug Something isn't working 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Feb 5, 2021
@davidwendt davidwendt self-assigned this Feb 5, 2021
@davidwendt
davidwendt requested a review from a team as a code owner February 5, 2021 21:55
@davidwendt davidwendt changed the title Fix invalid-device-fn cudf::strings::replace_re with multiple regex's Fix invalid-device-fn error in cudf::strings::replace_re with multiple regex's Feb 5, 2021
@codecov

codecov Bot commented Feb 6, 2021

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (branch-0.19@eb8dc88). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##             branch-0.19    #7336   +/-   ##
==============================================
  Coverage               ?   82.20%           
==============================================
  Files                  ?      100           
  Lines                  ?    16966           
  Branches               ?        0           
==============================================
  Hits                   ?    13947           
  Misses                 ?     3019           
  Partials               ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb8dc88...9a65618. Read the comment docs.

@nvdbaranec nvdbaranec left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

edit: forgot to remove this comment. Github won't let me delete for some reason.

Comment thread cpp/src/strings/replace/multi_re.cu Outdated
Comment thread cpp/src/strings/replace/multi_re.cu
@davidwendt
davidwendt requested a review from nvdbaranec February 8, 2021 20:29
@harrism harrism closed this Feb 9, 2021
@harrism

harrism commented Feb 9, 2021

Copy link
Copy Markdown
Contributor

@gpucibot merge

@ajschmidt8 ajschmidt8 reopened this Feb 10, 2021
@rapids-bot
rapids-bot Bot merged commit da3ab29 into NVIDIA:branch-0.19 Feb 10, 2021
@davidwendt
davidwendt deleted the build-gcc9 branch February 10, 2021 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants