Skip to content

DEV: Pass additional runtests.py args to ASV #15990

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

Merged
merged 1 commit into from
May 6, 2020

Conversation

seiko2plus
Copy link
Member

@seiko2plus seiko2plus commented Apr 15, 2020

Pass all extra arguments that start with -- to ASV

Examples:
runtests.py --bench-compare master sin --cpu-affinity 1
runtests.py --bench-compare master -- --cpu-affinity 1
runtests.py --bench sin --cpu-affinity 1
runtests.py --bench -- --cpu-affinity 1

@seberg
Copy link
Member

seberg commented Apr 15, 2020

Seems like this can be useful on its own, although I wonder if fixing -- together with --bench is not better. Maybe (although its not perfect probably):

diff --git a/runtests.py b/runtests.py
index e470f8a9d..2d9a0f39d 100755
--- a/runtests.py
+++ b/runtests.py
@@ -220,7 +220,14 @@ def main(argv):
 
     if args.bench:
         # Run ASV
-        items = extra_argv
+        if "--" in extra_argv:
+            split = extra_argv.index("--")
+            items = extra_argv[:split]
+            extra_argv = extra_argv[split+1:]
+        else:
+            items = extra_argv
+            extra_argv = []
+
         if args.tests:
             items += args.tests
         if args.submodule:
@@ -229,6 +236,7 @@ def main(argv):
         bench_args = []
         for a in items:
             bench_args.extend(['--bench', a])
+        bench_args.extend(extra_argv)
 
         if not args.bench_compare:
             cmd = ['asv', 'run', '-n', '-e', '--python=same'] + bench_args

plus maybe as an example in the initial help (it also doesn't list asv for --).

@seberg
Copy link
Member

seberg commented Apr 22, 2020

@seiko2plus if you think changing the script to pass -- is a good idea, do you want to do that? I assume nobody will mind, and since its an internal script, we can just finish this off quickly.

@seiko2plus seiko2plus force-pushed the asv_cpu_affinity branch 2 times, most recently from f4c3b22 to b3fccc8 Compare April 23, 2020 02:55
@seiko2plus seiko2plus changed the title ENH: Add new option to set CPU affinity for running the benchmark ENH:Bench pass all extra arguments that start with -- to ASV Apr 23, 2020
@seiko2plus seiko2plus changed the title ENH:Bench pass all extra arguments that start with -- to ASV ENH:Bench pass all extra arguments that start with '--' to ASV Apr 23, 2020
@seiko2plus
Copy link
Member Author

@seberg, alright I modified the patch with your proposal, it's more practical I guess. thank u!.

@seberg seberg self-requested a review April 23, 2020 17:57
@seberg
Copy link
Member

seberg commented May 2, 2020

@seiko2plus sorry, since this is a dev thing, I think we should just do it. I pushed a commit to make things work with -- (i.e. everything after a -- is passed on and the -- iself is skipped, but actual benchmarks must be listed before the --). That seemed the most reasonable thing to me, although I am not sure asv actually uses any non -- args, so it may not matter.

@seiko2plus
Copy link
Member Author

@seberg, your changes make it more robust since it allows passing all tokens after -- even the ones that start with an only single dash.

@seberg seberg closed this May 3, 2020
@seberg seberg reopened this May 3, 2020
@seberg
Copy link
Member

seberg commented May 3, 2020

For everyone else, since this only affects devlopment, and even then only the --bench option to runtests.py, I will just merge this soon I think.

EDIT: With that print removed obviously.

@seberg seberg force-pushed the asv_cpu_affinity branch from 55b7b76 to e04e090 Compare May 4, 2020 00:28
Since arguments list benchmarks, all arguments after the first
unknown `--` starting (removing a single first `--`) are passed
on to asv. Previously this was the right pattern for passing
to pytest, so it now works also for ASV.

Examples:
    `runtests.py` `--bench-compare master sin --cpu-affinity 1`
                  `--bench-compare master -- --cpu-affinity 1`
                  `--bench sin --cpu-affinity 1`
                  `--bench -- --cpu-affinity 1`
@seberg seberg force-pushed the asv_cpu_affinity branch from e04e090 to 0a99914 Compare May 4, 2020 00:30
@seberg seberg changed the title ENH:Bench pass all extra arguments that start with '--' to ASV DEV: Pass additional runtests.py args to ASV May 4, 2020
@seberg seberg merged commit e525a8f into numpy:master May 6, 2020
@seiko2plus
Copy link
Member Author

@seberg, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants