-
Notifications
You must be signed in to change notification settings - Fork 57
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
added median #19
base: master
Are you sure you want to change the base?
added median #19
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ | |
def run_benchmarks(control, experiment, benchmark_dir, benchmarks, trials, | ||
vcs=None, record_dir=None, profile_dir=None, | ||
continue_on_error=False, control_python=sys.executable, | ||
experiment_python=sys.executable): | ||
experiment_python=sys.executable, show_median=False): | ||
if benchmarks: | ||
print("Running benchmarks: %s" % " ".join(benchmarks)) | ||
else: | ||
|
@@ -104,7 +104,7 @@ def run_benchmarks(control, experiment, benchmark_dir, benchmarks, trials, | |
control_data=control_data, | ||
experiment_data=experiment_data, | ||
) | ||
print(format_benchmark_result(result, len(control_data.runtimes))) | ||
print(format_benchmark_result(result, len(control_data.runtimes), control_data.runtimes, experiment_data.runtimes, show_median)) | ||
print('') | ||
|
||
|
||
|
@@ -114,6 +114,13 @@ def discover_benchmarks(benchmark_dir): | |
os.path.exists(os.path.join(benchmark_dir, app, 'settings.py')): | ||
yield app | ||
|
||
def get_median(runs): | ||
sorted_runs = sorted(runs) | ||
middle = len(runs) / 2 | ||
if len(runs) % 2 == 0: | ||
return sum(sorted_runs[middle-1:middle+1])/2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I generally find this a little easier to read with spaces around the operators |
||
else: | ||
return sorted_runs[middle] | ||
|
||
def print_benchmarks(benchmark_dir): | ||
for app in discover_benchmarks(benchmark_dir): | ||
|
@@ -209,8 +216,7 @@ def insignificant(cls, text): | |
def bad(cls, text): | ||
return cls.colorize(cls.BAD, text) | ||
|
||
|
||
def format_benchmark_result(result, num_points): | ||
def format_benchmark_result(result, num_points, control_data, experiment_data, show_median): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering whether |
||
if isinstance(result, perf.BenchmarkResult): | ||
output = '' | ||
delta_min = result.delta_min | ||
|
@@ -227,6 +233,15 @@ def format_benchmark_result(result, num_points): | |
delta_avg = colorize.bad(delta_avg) | ||
output += "Avg: %f -> %f: %s\n" % (result.avg_base, result.avg_changed, delta_avg) | ||
|
||
if show_median: | ||
median = get_median(control_data) - get_median(experiment_data) | ||
median_text = "%0.10f" % median | ||
if median > .0: | ||
median_body = colorize.good(median_text) | ||
else: | ||
median_body = colorize.bad(median_text) | ||
output += "Median: %s\n" % median_body | ||
|
||
t_msg = result.t_msg | ||
if 'Not significant' in t_msg: | ||
t_msg = colorize.insignificant(t_msg) | ||
|
@@ -366,6 +381,14 @@ def main(): | |
action='store_true', | ||
help='List all available benchmarks and exit.', | ||
) | ||
parser.add_argument( | ||
'-m', | ||
'--median', | ||
dest='show_median', | ||
default=False, | ||
action='store_true', | ||
help='Show median in resulting benchmarks', | ||
) | ||
parser.add_argument( | ||
'--log', | ||
dest='loglevel', | ||
|
@@ -395,6 +418,7 @@ def main(): | |
continue_on_error=args.continue_on_error, | ||
control_python=args.control_python, | ||
experiment_python=args.experiment_python, | ||
show_median=args.show_median, | ||
) | ||
|
||
if __name__ == '__main__': | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is getting pretty long - perhaps it'd be best to break it on the new trailing comma so it'd be easier to scan – maybe like this?