Skip to content

Conversation

wphicks
Copy link
Contributor

@wphicks wphicks commented Dec 8, 2020

Replace "constexpr static" member variables in DecisionTree unit test fixture with "const" member variables for compliance with C++14, which otherwise requires that const static data members be separately defined in a namespace scope if it is ODR-used (See sections 3.2 and 9.4.2 of the C++11 standard, which remain relevant until C++17).

Replace "constexpr static" member variables in DecisionTree unit test
fixture with "const" member variables for compliance with C++14, which
otherwise requires that static data members be separately defined in a
namespace scope if it is ODR-used
@wphicks wphicks requested a review from a team as a code owner December 8, 2020 23:13
@wphicks wphicks added 3 - Ready for Review Ready for review by team bug Something isn't working non-breaking Non-breaking change labels Dec 8, 2020
@wphicks
Copy link
Contributor Author

wphicks commented Dec 8, 2020

Together with PR #3279, this resolves #3212, allowing the debug build to compile and link correctly once more.

@wphicks
Copy link
Contributor Author

wphicks commented Dec 8, 2020

Oh, and for those wondering why this links at all in the non-debug build, it is because these are optimized as inline constants there.

@wphicks
Copy link
Contributor Author

wphicks commented Dec 9, 2020

The failures do not seem to be related to this change. The failures were in:

  • cuml.test.stemmer_tests.test_stemmer.test_same_results
  • cuml.test.stemmer_tests.test_steps.test_step5b

I'll rerun and if everything seems alright, I'll submit issues for those two.

@wphicks
Copy link
Contributor Author

wphicks commented Dec 9, 2020

rerun tests

Copy link
Member

@dantegd dantegd left a comment

Choose a reason for hiding this comment

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

lgtm

Bring in changes to avoid issue with stemmer test in CI
@wphicks
Copy link
Contributor Author

wphicks commented Dec 15, 2020

Merged in changes from 0.18 to eliminate failing test

@codecov-io
Copy link

codecov-io commented Dec 15, 2020

Codecov Report

Merging #3281 (4ddbd21) into branch-0.18 (2e4388d) will increase coverage by 0.23%.
The diff coverage is 92.85%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.18    #3281      +/-   ##
===============================================
+ Coverage        71.45%   71.69%   +0.23%     
===============================================
  Files              205      207       +2     
  Lines            16594    16884     +290     
===============================================
+ Hits             11858    12105     +247     
- Misses            4736     4779      +43     
Impacted Files Coverage Δ
python/cuml/neighbors/__init__.py 100.00% <ø> (ø)
python/cuml/neighbors/ann.pxd 87.05% <87.05%> (ø)
python/cuml/common/sparsefuncs.py 91.66% <88.23%> (-2.34%) ⬇️
python/cuml/linear_model/base.pyx 97.36% <97.36%> (ø)
python/cuml/neighbors/nearest_neighbors.pyx 92.36% <98.24%> (+1.90%) ⬆️
python/cuml/linear_model/elastic_net.pyx 88.00% <100.00%> (-0.89%) ⬇️
python/cuml/linear_model/lasso.pyx 90.47% <100.00%> (-0.83%) ⬇️
python/cuml/linear_model/linear_regression.pyx 87.95% <100.00%> (-1.68%) ⬇️
python/cuml/linear_model/ridge.pyx 87.09% <100.00%> (-1.70%) ⬇️
python/cuml/manifold/t_sne.pyx 76.07% <100.00%> (+0.95%) ⬆️
... and 8 more

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 34efaf8...4ddbd21. Read the comment docs.

@wphicks
Copy link
Contributor Author

wphicks commented Dec 15, 2020

rerun tests

1 similar comment
@wphicks
Copy link
Contributor Author

wphicks commented Dec 17, 2020

rerun tests

@wphicks wphicks linked an issue Dec 17, 2020 that may be closed by this pull request
@rapids-bot rapids-bot bot merged commit ae7e444 into rapidsai:branch-0.18 Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Debug build results in compile error
3 participants