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

support space in workspace #3752

Merged
merged 5 commits into from
Aug 18, 2021
Merged

support space in workspace #3752

merged 5 commits into from
Aug 18, 2021

Conversation

jolorunyomi
Copy link
Contributor

Summary

This PR adds support for space to workspace

@jolorunyomi jolorunyomi requested a review from a team as a code owner April 15, 2021 19:11
@github-actions github-actions bot added the gpuCI gpuCI issue label Apr 15, 2021
ajschmidt8
ajschmidt8 requested changes Apr 16, 2021
View reviewed changes
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

looks like there's a lot of unintended changes in this PR. There are others besides the ones I highlighted, so please manually review the entire PR again before re-requesting reviews.

ci/cpu/build.sh Outdated Show resolved Hide resolved
ci/gpu/build.sh Outdated Show resolved Hide resolved
ci/gpu/build.sh Outdated Show resolved Hide resolved
ci/gpu/build.sh Outdated Show resolved Hide resolved
ci/gpu/build.sh Outdated Show resolved Hide resolved
@github-actions
Copy link

This PR has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates. This PR will be labeled inactive-90d if there is no activity in the next 60 days.

@jolorunyomi jolorunyomi added 2 - In Progress Currenty a work in progress improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed inactive-30d labels May 25, 2021
ajschmidt8
ajschmidt8 requested changes May 25, 2021
View reviewed changes
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

there's a lot of errors in this PR. please review all of the changes entirely before re-requesting a review.

ci/gpu/build.sh Outdated
Comment on lines 123 to 124
GTE"$WORKSPACE/cpp/build"KSPACE}/test-results/libcuml_cpp/" ./test/ml
"$WORKSPACE/test-results/libcuml_cpp/"
Copy link
Member

Choose a reason for hiding this comment

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

this line needs to be fixed.

ci/gpu/build.sh Outdated
Comment on lines 128 to 130
gpu"$WORKSPACE/python" pytest for cuml"
cd $WORKSPACE/python

"$WORKSPACE/cuml-cuda-tmp""$WORKSPACE/junit-cuml.xml""$WORKSPACE/python/cuml/cuml-coverage.xml"
Copy link
Member

Choose a reason for hiding this comment

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

these lines should be fixed.

ci/gpu/build.sh Outdated
pytest --cache-clear --basetemp=${WORKSPACE}/cuml-cuda-tmp --junitxml=${WORKSPACE}/junit-cuml.xml -v -s -m "not memleak" --durations=50 --timeout=300 --ignore=cuml/test/dask --ignore=cuml/raft --cov-config=.coveragerc --cov=cuml --cov-report=xml:${WORKSPACE}/python/cuml/cuml-coverage.xml --cov-report term

"$WORKSPACE/cuml-mg-cuda-tmp""$WORKSPACE/junit-cuml-mg.xml"
Copy link
Member

Choose a reason for hiding this comment

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

should this line have been added?

jolorunyomi
jolorunyomi commented May 25, 2021
View reviewed changes
ci/gpu/build.sh Outdated
Comment on lines 123 to 124
GTE"$WORKSPACE/cpp/build"KSPACE}/test-results/libcuml_cpp/" ./test/ml
"$WORKSPACE/test-results/libcuml_cpp/"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
GTE"$WORKSPACE/cpp/build"KSPACE}/test-results/libcuml_cpp/" ./test/ml
"$WORKSPACE/test-results/libcuml_cpp/"
GTEST_OUTPUT="xml:${WORKSPACE}/test-results/libcuml_cpp/" ./test/ml

jolorunyomi
jolorunyomi commented May 25, 2021
View reviewed changes
ci/gpu/build.sh Outdated
export LD_LIBRARY_PATH=$LD_LIBRARY_PATH_CACHED
export LD_LIBRARY_PATH_CACHED=""

gpuci_logger "Python pytest for cuml"
gpu"$WORKSPACE/python" pytest for cuml"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
gpu"$WORKSPACE/python" pytest for cuml"
gpuci_logger "Python pytest for cuml"

jolorunyomi
jolorunyomi commented May 25, 2021
View reviewed changes
ci/gpu/build.sh Outdated
cd $WORKSPACE/python

"$WORKSPACE/cuml-cuda-tmp""$WORKSPACE/junit-cuml.xml""$WORKSPACE/python/cuml/cuml-coverage.xml"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"$WORKSPACE/cuml-cuda-tmp""$WORKSPACE/junit-cuml.xml""$WORKSPACE/python/cuml/cuml-coverage.xml"

jolorunyomi
jolorunyomi commented May 25, 2021
View reviewed changes
ci/gpu/build.sh Outdated
pytest --cache-clear --basetemp=${WORKSPACE}/cuml-cuda-tmp --junitxml=${WORKSPACE}/junit-cuml.xml -v -s -m "not memleak" --durations=50 --timeout=300 --ignore=cuml/test/dask --ignore=cuml/raft --cov-config=.coveragerc --cov=cuml --cov-report=xml:${WORKSPACE}/python/cuml/cuml-coverage.xml --cov-report term

"$WORKSPACE/cuml-mg-cuda-tmp""$WORKSPACE/junit-cuml-mg.xml"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"$WORKSPACE/cuml-mg-cuda-tmp""$WORKSPACE/junit-cuml-mg.xml"

@jolorunyomi
Copy link
Contributor Author

These must have occurred as a result of a shift in target of base branch, I'll re-work this PR in a moment.

ajschmidt8
ajschmidt8 requested changes May 26, 2021
View reviewed changes
ci/docs/build.sh Outdated Show resolved Hide resolved
@dantegd
Copy link
Member

dantegd commented May 28, 2021

@ajschmidt8 @jolorunyomi do we still want to target this to 21.06?

@jolorunyomi @ajschmidt8
Update ci/docs/build.sh
0a491c2 
Co-authored-by: AJ Schmidt <ajschmidt8@users.noreply.github.com>
ajschmidt8
ajschmidt8 requested changes Jun 1, 2021
View reviewed changes
ci/docs/build.sh Outdated Show resolved Hide resolved
@ajschmidt8
Copy link
Member

@ajschmidt8 @jolorunyomi do we still want to target this to 21.06?

Yes, I think this should be fine to include in 21.06 as long as it makes it in before codefreeze.

@jolorunyomi @ajschmidt8
Update ci/docs/build.sh
0822b56 
Co-authored-by: AJ Schmidt <ajschmidt8@users.noreply.github.com>
@ajschmidt8
Copy link
Member

@jolorunyomi, please fix the style check issues. View the logs in the GitHub Check for more details

@raydouglass raydouglass changed the base branch from branch-21.06 to branch-21.08 June 2, 2021 20:22
@caryr35 caryr35 changed the base branch from branch-21.08 to branch-21.10 July 16, 2021 20:34
@jolorunyomi
Copy link
Contributor Author

Style checks are failing for unrelated copyright reasons.

@jolorunyomi jolorunyomi merged commit 7f0f208 into branch-21.10 Aug 18, 2021
@jolorunyomi jolorunyomi deleted the workspace branch August 18, 2021 12:56
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
@jolorunyomi
Merge pull request rapidsai#3752 from rapidsai/workspace
2ada4ed 
support space in workspace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

Assignees
No one assigned
Labels
2 - In Progress Currenty a work in progress gpuCI gpuCI issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jolorunyomi @dantegd @ajschmidt8