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

Avoid the use of shared variable. #50

Merged
merged 7 commits into from
Aug 19, 2024
Merged

Conversation

trivialfis
Copy link
Member

@trivialfis trivialfis commented Aug 19, 2024

  • Use register instead.
  • Fix Python benchmark script with latest pandas and XGBoost.

I ran the Python benchmark script using RTX8000 with CPU excluded:

PR

model test_rows cpu_time(s) cpu_std gpu_time(s) gpu_std speedup
adult-small 10000 0.0 0.011279034591279925 0.0071617705732274824
adult-med 10000 0.0 0.11670323801226914 0.011867081750644454
adult-large 10000 0.0 9.429356226988602 0.047229702183655395
covtype-small 10000 0.0 0.01151795060141012 0.003561754665202217
covtype-med 10000 0.0 0.6641728987917304 0.0032803489983452214
covtype-large 10000 0.0 74.24905845939648 0.5906834845783823
cal_housing-small 10000 0.0 0.006276315008290112 0.002279411050170562
cal_housing-med 10000 0.0 0.10911295339465141 0.0012658547831405737
cal_housing-large 10000 0.0 23.429275520995724 0.08867194402118761
fashion_mnist-small 10000 0.0 0.16429676760453732 0.06816532400946669
fashion_mnist-med 10000 0.0 4.30540003181668 0.07100682231932201
fashion_mnist-large 10000 0.0 178.59231641321676 1.0623992933829536

main

model test_rows cpu_time(s) cpu_std gpu_time(s) gpu_std speedup
adult-small 10000 0.0 0.010922096599824727 0.006964535125901965
adult-med 10000 0.0 0.11171154719777406 0.012917956737291719
adult-large 10000 0.0 9.12171239000745 0.16260832207140813
covtype-small 10000 0.0 0.011427397408988326 0.003081750313730569
covtype-med 10000 0.0 0.6582016726024449 0.010074246049065491
covtype-large 10000 0.0 73.67104159318842 0.6321216129573327
cal_housing-small 10000 0.0 0.005807545990683139 0.00245267488361271
cal_housing-med 10000 0.0 0.10978057580068708 0.0016856217806583914
cal_housing-large 10000 0.0 23.66984584759921 0.10060247707348238
fashion_mnist-small 10000 0.0 0.1685630762251094 0.06951830276227733
fashion_mnist-med 10000 0.0 4.44043413939653 0.06073396821402969
fashion_mnist-large 10000 0.0 183.3073539715959 0.6199842390017297

- Use register instead.
- Fix Python benchmark script.
@trivialfis trivialfis requested a review from a team as a code owner August 19, 2024 05:41
@trivialfis trivialfis requested a review from a team as a code owner August 19, 2024 07:48
@trivialfis trivialfis requested a review from raydouglass August 19, 2024 07:48
@trivialfis
Copy link
Member Author

trivialfis commented Aug 19, 2024

@hcho3 Have you seen this error using the rapids test matrix generator:

  File "/opt/conda/lib/python3.11/site-packages/rapids_dependency_file_generator/_rapids_dependency_file_generator.py", line 421, in make_dependency_files
    raise ValueError(f"No matching matrix found in '{include}' for: {matrix_combo}")
ValueError: No matching matrix found in 'cuda_version' for: {'cuda': '12.5', 'arch': 'x86_64', 'py': '3.11'}

with rapidsai/shared-workflows/.github/workflows/conda-cpp-tests.yaml@branch-24.08?

.github/workflows/pr.yaml Outdated Show resolved Hide resolved
trivialfis and others added 3 commits August 20, 2024 00:21
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
@bdice bdice requested a review from a team as a code owner August 19, 2024 16:24
@bdice
Copy link
Contributor

bdice commented Aug 19, 2024

The failure on this PR is due to the addition of CUDA 12.5 support. This repository was missed during the upgrade. rapidsai/build-planning#73

I went ahead and pushed the changes you need in 444005e.

@bdice
Copy link
Contributor

bdice commented Aug 19, 2024

I also pushed commit e04ea9b with the changes from ci/release/update-version.sh 24.10.00. This repo is updated pretty irregularly and we need to keep all the RAPIDS versions aligned (not just the CI workflow branches).

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Approving packaging changes. Thanks for the fix @trivialfis. Hope it's alright with the team that I am approving my own fixes, nothing should be controversial here (just missed version updates) and should unblock CI.

@trivialfis trivialfis merged commit 40eae8c into rapidsai:main Aug 19, 2024
6 checks passed
@trivialfis trivialfis deleted the avoid-shared branch August 19, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants