Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

BUG, Benchmark: fix passing optimization build options to asv#17736

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
mattip merged 1 commit intonumpy:masterfromseiko2plus:issue_17716
Dec 16, 2020

Conversation

seiko2plus
Copy link
Member

@seiko2plusseiko2plus commentedNov 9, 2020
edited
Loading

Fix passing optimization build options to asv

closes#17716

see#17716 (comment)

@Qiyu8
Copy link
Member

I tested your code in two strategies:

  • single branch benchmark: create new branch, download your code, run the following command.

    python runtests.py --cpu-baseline="sse2" --bench bench_linalg

    • TheNPY_SIMDis 128.

    python runtests.py --cpu-baseline="avx2" --bench bench_linalg

    • TheNPY_SIMDis 256.
  • two branch benchmark: create two new branch, download your code in both branch, run the following command.

    python runtests.py --cpu-baseline="sse2" --bench-compare test_asv bench_linalg

    • TheNPY_SIMDis 128.

    python runtests.py --cpu-baseline="avx2" --bench-compare test_asv bench_linalg

    • TheNPY_SIMDis 256.

To sum it up, the code here works fine in my computer. Thanks@seiko2plus .

@seiko2plus
Copy link
MemberAuthor

@Qiyu8, thank you for reporting, you can also count on#17737 to verify the supported features. it provides the same format that used within pytest.

"PIP_NO_BUILD_ISOLATION=false python -mpip wheel --no-deps --no-index -w {build_cache_dir} {build_dir}"
// pip ignores '--global-option' when pep517 is enabled, we also enabling pip verbose to
// be reached from asv `--verbose` so we can verify the build options.
"PIP_NO_BUILD_ISOLATION=false python {build_dir}/benchmarks/asv_pip_nopep517.py -v {numpy_global_options} --no-deps --no-index -w {build_cache_dir} {build_dir}"
Copy link
Member

Choose a reason for hiding this comment

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

This should be broken into two steps: one to check that--no-use-pep517 is supported, and one to build the wheel. It might be easier to avoid the pip build altogether by adding abdist_wheel build step

rm -rf distpython setup.py build {numpy_build_options} bdist_wheelpython -mpip install dist/*.whl

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

This should be broken into two steps: one to check that --no-use-pep517 is supported, and one to build the wheel

already handle it through a separate python scriptasv_pip_nopep517.py, we're not free to use POSIX commands since we're targeting non-unix platforms too. e.g. windows

It might be easier to avoid the pip build altogether by adding a bdist_wheel build step

I tried to usebdist_wheel but ASV wouldn't able to fetch the wheel file and cache it.
also, I tried to only use thepip command only but idk whyccache doesn't work properly

here is the default values of build commands :

"install_command":    ["python -mpip install {wheel_file}"],"uninstall_command":    ["return-code=any python -mpip uninstall -y {project}"],"build_command":    ["python setup.py build","PIP_NO_BUILD_ISOLATION=false python -mpip wheel --no-deps --no-index -w {build_cache_dir} {build_dir}"]

@Qiyu8
Copy link
Member

I think this PR is good to merge and will unlock other performance optimizations.

@mattip
Copy link
Member

While I do not debate the value in passing optimization flags to asv, I think this PR still is not ready. There should be a simpler and more direct way to get interaction between asv and code building.

Here are some ideas, it may be worth discussing them more widely:

@seiko2plus
Copy link
MemberAuthor

seiko2plus commentedNov 12, 2020
edited
Loading

@mattip,

There should be a simpler and more direct way to get interaction between asv and code building.

unfortunatelyasv continuous doesn't offer any other way, checkairspeed-velocity/asv#803

Here are some ideas, it may be worth discussing them more widely

First, we need to take a decision to either accepting this pull request as a workaround or close it and revert#17284

overhaul runtests.py and refactor it into proper subcommands rather than using home-grown argparse enhancements.

as a start, we can create an independent scriptbenchmark.py that mapping all important args to ASV, it should
provides similar functionality togh-15987

enhance ASV and/or the interaction between the benchmarks, ASV, and NumPy. PRsgh-17737,gh-15987 andgh-15992 are also related to this.

Here is the issue, ASV is based on command arguments that kill the flexibility.
I vote to create our own micro-benchmarking tool and drop ASV.

EDIT: I mentioned a wrong pr

@Qiyu8
Copy link
Member

I think It's hard time to dropasvsupport, because this benchmark toolkit is working well among normal modules, The currentargbased simd mechanism exposed the drawbacks ofasv, which should improved throughcommunity.

@seiko2plus
Copy link
MemberAuthor

@Qiyu8, with all respect, we can't count on another project or community in every single new feature. I seegoogle benchmark as a good alternative, we can build our new tool on top of it. It already provides decent flexibility and been used by many projects.

@seiko2plus
Copy link
MemberAuthor

@mattip, there's no other way to get ride of it without this workaround. ugly but necessary and the same thing for#17737.
please I need to hear your final call.

@mattipmattip added 01 - Enhancement triage reviewIssue/PR to be discussed at the next triage meeting labelsDec 9, 2020
@mattip
Copy link
Member

I marked this for triage-review so we can get some more opinions.

@seiko2plus
Copy link
MemberAuthor

@mattip, thank you Matti, I really appreciate your help.

@mattip
Copy link
Member

The SIMD optimizations is stretching the current workflows:

  • a need to benchmark ufunc inner loops
  • a new templating engine for C code
  • a new compiler option extension for setting machine options to C flags (merged)

Maybe we should think a bit about separating the loops out into a separate library that would be easier to test and benchmark, along the lines of issuegh-17698.

@mattip
Copy link
Member

In the triage meeting we decided to merge this, but would like to rethink benchmarking in general. That is a different discussion though, so in this goes.

@mattipmattip merged commitd7a75e8 intonumpy:masterDec 16, 2020
@mattip
Copy link
Member

Thanks@seiko2plus

@seiko2plus
Copy link
MemberAuthor

@mattip,

Infrastructure, SIMD, and GPU optimizations must be part of the core of any modern computing library, branching the core or counting on other projects will badly affect workflow, especially for big projects.

We have something new here, let's give it the full chance before thinking of separating it into a separate repo. I think NumPy should find a foot in highly computing processing instead of playing the mediator role, in order to maintain its popularity.

need to benchmark ufunc inner loops

We need also a tool for sanity data check too. OpenCV has 2 in 1 tool benchmarking, and sanity checks at the same time. seeHowToUsePerfTests very effective in detecting any deviations.

a new templating engine for C code

Why do we need a new engine and we have python? we just need a better way to using it in our C sources. I think#17952 can be a good solution.

In the triage meeting we decided to merge this,

Thank you a lot.

but would like to rethink benchmarking in general.

I'm not sure what we should do. To create our own minimal tool that serving our only needs or contributing to ASV. What do you prefer?

@mattip
Copy link
Member

In general, I do not like re-inventing the wheel. If a tool or paradigm can be adapted to our needs, then that is preferable over creating our own, unless our needs are very different.

I think NumPy should find a foot in highly computing processing instead of playing the mediator role, in order to maintain its popularity.

I disagree. If by splitting out the great work you have done with SIMD we enable others to reuse it, that would be progress.

What do you prefer?

It seems like right now ASV and pyperf are very similar, but neither serves our needs very well. I think we should explore ways to make ASV better before creating our own tool. Making those tools better would help others who are also struggling with benchmarking.

@rossbar
Copy link
Contributor

It seems like right now ASV and pyperf are very similar, but neither serves our needs very well. I think we should explore ways to make ASV better before creating our own tool. Making those tools better would help others who are also struggling with benchmarking.

Well said, I very much agree 👍

@seiko2plus
Copy link
MemberAuthor

@mattip, I will give ASV a try I guess we have enough burden and no need to open a new front.

@seiko2plusseiko2plus deleted the issue_17716 branchJanuary 9, 2021 16:52
@rgommersrgommers added the component: SIMDIssues in SIMD (fast instruction sets) code or machinery labelJul 12, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@mattipmattipmattip left review comments

Assignees
No one assigned
Labels
00 - Bug01 - Enhancementcomponent: SIMDIssues in SIMD (fast instruction sets) code or machinerytriage reviewIssue/PR to be discussed at the next triage meeting
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SIMD: The --cpu-baseline is not correctly configured in benchmark system.
5 participants
@seiko2plus@Qiyu8@mattip@rossbar@rgommers

[8]ページ先頭

©2009-2025 Movatter.jp