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

add some modification to support torchbench-cpu well #795

Merged
merged 1 commit into from
Dec 12, 2022
Merged

add some modification to support torchbench-cpu well #795

merged 1 commit into from
Dec 12, 2022

Conversation

Guo-Peilin
Copy link
Collaborator

No description provided.

wyzero
wyzero reviewed Nov 25, 2022
View reviewed changes
pytorch_blade/benchmark/TorchBench/test_torch_bench.sh Outdated
export OMP_NUM_THREADS=1 GOMP_CPU_AFFINITY=4
else
# eg: bash test_torch_bench.sh cpu tiny 32 "0-63:2"
export OMP_NUM_THREADS=$3 GOMP_CPU_AFFINITY=$4
Copy link
Collaborator

Choose a reason for hiding this comment

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

not enough if only limiting OMP threads since the framework (e.g. torch/TF) may have its own thread pool as well. maybe be use taskset as instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

.github/workflows/torchbench.yml Outdated
secrets: inherit
TorchBenchCpuFull8Threads:
Copy link
Collaborator

Choose a reason for hiding this comment

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

better to encode CPU type into the name as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right now cpu means x86 only, aarch64 has itw own name

pytorch_blade/benchmark/TorchBench/test_torch_bench.sh Outdated
results=(eval-cpu-fp32)
else
results=(eval-cuda-fp32 eval-cuda-fp16)
fi
python3 torchbenchmark/onnxrt_helper.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

note that latest ORT may use its own thread pool instead of ORT, we may need extra thread related setting for it. This can be leaved for future improvement.

wyzero
wyzero reviewed Nov 28, 2022
View reviewed changes
pytorch_blade/benchmark/TorchBench/test_torch_bench.sh Outdated

config_file=blade_$1_$2.yaml
bench_target=$2
binding_cores=1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using 0 as default is better in case there is only 1 core used for testing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

pytorch_blade/benchmark/TorchBench/test_torch_bench.sh Outdated
export GOMP_CPU_AFFINITY="2-5"
if [ ! -n "$3" ]
then
export OMP_NUM_THREADS=1 GOMP_CPU_AFFINITY=4
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why using different default value for GOMP_CPU_AFFINITY and taskset?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the origin code using in this way. Now I using the same value

wyzero
wyzero previously approved these changes Nov 28, 2022
View reviewed changes
qiuxiafei
qiuxiafei reviewed Nov 28, 2022
View reviewed changes
pytorch_blade/benchmark/TorchBench/test_torch_bench.sh Outdated

if [ $1 == "cpu" ]
then
# 4 cores
export GOMP_CPU_AFFINITY="2-5"
if [ ! -n "$3" ]
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: ! -n is identical to -z

pytorch_blade/benchmark/TorchBench/test_torch_bench.sh Outdated
results=(eval-cpu-fp32)
taskset -c $bingding_cores python3 torchbenchmark/.github/scripts/run-config.py \
Copy link
Collaborator

Choose a reason for hiding this comment

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

misspelled bingding_cores ...

@zzpmiracle
Copy link
Collaborator

Manually trigger this workflow in https://github.com/alibaba/BladeDISC/actions/workflows/torchbench.yml ?

@Guo-Peilin Guo-Peilin force-pushed the support-torchbench-on-cpu branch 6 times, most recently from 50d7966 to d82a075 Compare November 30, 2022 11:57
zzpmiracle
zzpmiracle reviewed Nov 30, 2022
View reviewed changes
.github/workflows/torchbench.yml Outdated
base_image: bladedisc/bladedisc:latest-runtime-torch1.12.0-cpu
device: cpu_benchmark
dockerfile: docker/cronjobs/Dockerfile.torch.bench
extra_envs: -e RELATED_DIFF_PERCENT=3
Copy link
Collaborator

Choose a reason for hiding this comment

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

diff has changed to 5%

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

.github/workflows/torchbench.yml Outdated
dockerfile: docker/cronjobs/Dockerfile.torch.bench
extra_envs: -e RELATED_DIFF_PERCENT=3
exec_command: bash ./pytorch_blade/benchmark/TorchBench/test_torch_bench.sh cpu full 8 "0-7"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can loop for different configs in one job, not create jobs for each config.

@Guo-Peilin Guo-Peilin force-pushed the support-torchbench-on-cpu branch 12 times, most recently from 22aa842 to b85c8d7 Compare December 5, 2022 11:57
zzpmiracle
zzpmiracle reviewed Dec 5, 2022
View reviewed changes
.github/workflows/torchbench.yml Outdated
name: torch-offcial-benchmark
base_image: bladedisc/bladedisc:latest-runtime-torch1.12.0-cpu-aarch64
device: cpu_benchmark
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since not use extra machine for benchmark, we can directly use aarch64 / cpu for aarch64 / cpu benchmark now.

pytorch_blade/benchmark/TorchBench/test_torch_bench.sh Show resolved Hide resolved
@Guo-Peilin Guo-Peilin force-pushed the support-torchbench-on-cpu branch 10 times, most recently from 0993257 to 829f0ba Compare December 8, 2022 06:47
@Guo-Peilin
Copy link
Collaborator Author

Guo-Peilin commented Dec 8, 2022 •
edited
Loading

image

image

zzpmiracle
zzpmiracle reviewed Dec 9, 2022
View reviewed changes
pytorch_blade/benchmark/TorchBench/test_torch_bench.sh Outdated
@@ -21,6 +24,9 @@ fi
# for CI git-lfs permission problems
pushd $benchmark_repo_dir
# cache venv in benchmark dir
if [ $1 == "aarch64" ]; then
rm -rf ./venv && cp -r /opt/venv_disc ./venv
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe not remove this folder to cache venv

zzpmiracle
zzpmiracle approved these changes Dec 12, 2022
View reviewed changes
Copy link
Collaborator

@zzpmiracle zzpmiracle left a comment

Choose a reason for hiding this comment

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

LGTM!

@Guo-Peilin Guo-Peilin merged commit 512a8dc into main Dec 12, 2022
@Guo-Peilin Guo-Peilin deleted the support-torchbench-on-cpu branch December 12, 2022 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiuxiafei qiuxiafei qiuxiafei left review comments

@zzpmiracle zzpmiracle zzpmiracle approved these changes

@wyzero wyzero wyzero left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Guo-Peilin @zzpmiracle @qiuxiafei @wyzero