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

Patch for nightly test&bench #4840

Merged

Conversation

viclafargue
Copy link
Contributor

@viclafargue viclafargue commented Jul 29, 2022

@viclafargue viclafargue requested review from a team as code owners July 29, 2022 12:57
@github-actions github-actions bot added CUDA/C++ Cython / Python Cython or Python issue labels Jul 29, 2022
@caryr35 caryr35 added this to PR-WIP in v22.10 Release via automation Aug 4, 2022
@caryr35 caryr35 moved this from PR-WIP to PR-Needs review in v22.10 Release Aug 4, 2022
@github-actions github-actions bot added CMake conda conda issue gpuCI gpuCI issue labels Aug 11, 2022
@viclafargue viclafargue requested review from a team as code owners August 12, 2022 12:26
@ajschmidt8
Copy link
Member

is this PR going into 22.08 or 22.10? The base branch says 22.08 right now.

@viclafargue viclafargue changed the base branch from branch-22.08 to branch-22.10 August 15, 2022 14:40
@viclafargue
Copy link
Contributor Author

is this PR going into 22.08 or 22.10? The base branch says 22.08 right now.

Just changed it to 22.10

@@ -263,6 +266,7 @@ cdef class ForestInference_impl():
self.handle = handle
self.forest_data = forest_variant(<forest32_t> NULL)
self.shape_str = NULL
self.mr = get_current_device_resource()
Copy link
Contributor

Choose a reason for hiding this comment

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

When is self.mr used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some Python models are storing rmm::device_vector and other RMM objects or structs to containing these objects. These only have a pointer to their respective memory resources. Then depending on garbage collection, the memory resource (see memory_resource.pyx) can be released before objects depending on it for CUDA deallocation. This sometimes results in a segfault (visible when benchmarking for instance). Keeping a reference to the memory resource inside the model prevents its premature release (e.g. : in DeviceBuffer).

@github-actions github-actions bot removed gpuCI gpuCI issue conda conda issue CMake labels Aug 19, 2022
@ajschmidt8 ajschmidt8 removed the request for review from a team August 26, 2022 16:11
@lowener
Copy link
Contributor

lowener commented Aug 29, 2022

rerun tests

2 similar comments
@dantegd
Copy link
Member

dantegd commented Aug 30, 2022

rerun tests

@dantegd
Copy link
Member

dantegd commented Aug 30, 2022

rerun tests

@dantegd dantegd added bug Something isn't working non-breaking Non-breaking change labels Aug 30, 2022
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.

codeowner approval

v22.10 Release automation moved this from PR-Needs review to PR-Reviewer approved Sep 8, 2022
@dantegd
Copy link
Member

dantegd commented Sep 8, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit f0fa3a3 into rapidsai:branch-22.10 Sep 8, 2022
v22.10 Release automation moved this from PR-Reviewer approved to Done Sep 8, 2022
jakirkham pushed a commit to jakirkham/cuml that referenced this pull request Feb 27, 2023
- Fix for MNMG TSVD (similar issue to [cudaErrorContextIsDestroyed in RandomForest](rapidsai#2632 (comment)))
- rapidsai#4826
- MNMG Kmeans testing issue : modification of accuracy threshold
- MNMG KNNRegressor testing issue : modification of input for testing
- LabelEncoder documentation test issue : modification of pandas/cuDF display configuration
- RandomForest testing issue : adjust number of estimators to the number of workers

Authors:
  - Victor Lafargue (https://github.com/viclafargue)

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: rapidsai#4840
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CUDA/C++ Cython / Python Cython or Python issue non-breaking Non-breaking change
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants