-
Notifications
You must be signed in to change notification settings - Fork 121
Tolerances for CI Workflow - Grind & Exec Times (#750) #876
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
Changes from 9 commits
5c4eb09
96aa00b
75af5e4
1522ddf
3221273
bb22a53
6566179
6335fc0
18c10f7
15d1d75
38c70dd
3e6c917
4e1eacc
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 |
|---|---|---|
|
|
@@ -56,11 +56,11 @@ def bench(targets = None): | |
|
|
||
| with open(log_filepath, "w") as log_file: | ||
| system( | ||
| ["./mfc.sh", "run", case.path, "--case-optimization"] + | ||
| ["./mfc.sh", "run", case.path] + | ||
| ["--targets"] + [t.name for t in targets] + | ||
| ["--output-summary", summary_filepath] + | ||
| case.args + | ||
| ["--", "--gbpp", ARG('mem')], | ||
| ["--", "--gbpp", 0.5], | ||
| stdout=log_file, | ||
| stderr=subprocess.STDOUT) | ||
|
|
||
|
|
@@ -81,11 +81,10 @@ def bench(targets = None): | |
| # pylint: disable=too-many-branches | ||
| def diff(): | ||
| lhs, rhs = file_load_yaml(ARG("lhs")), file_load_yaml(ARG("rhs")) | ||
|
|
||
| cons.print(f"[bold]Comparing Benchmarks: Speedups from [magenta]{os.path.relpath(ARG('lhs'))}[/magenta] to [magenta]{os.path.relpath(ARG('rhs'))}[/magenta] are displayed below. Thus, numbers > 1 represent increases in performance.[/bold]") | ||
|
|
||
| if lhs["metadata"] != rhs["metadata"]: | ||
| def _lock_to_str(lock): | ||
| return ' '.join([f"{k}={v}" for k, v in lock.items()]) | ||
| _lock_to_str = lambda lock: ' '.join([f"{k}={v}" for k, v in lock.items()]) | ||
|
|
||
| cons.print(f"""\ | ||
| [bold yellow]Warning[/bold yellow]: Metadata in lhs and rhs are not equal. | ||
|
|
@@ -114,56 +113,39 @@ def _lock_to_str(lock): | |
| table.add_column("[bold]Post Process[/bold]", justify="right") | ||
|
|
||
| err = 0 | ||
|
|
||
| for slug in slugs: | ||
| lhs_summary = lhs["cases"][slug]["output_summary"] | ||
| rhs_summary = rhs["cases"][slug]["output_summary"] | ||
|
|
||
| lhs_summary, rhs_summary = lhs["cases"][slug]["output_summary"], rhs["cases"][slug]["output_summary"] | ||
| speedups = ['N/A', 'N/A', 'N/A'] | ||
|
|
||
| for i, target in enumerate(sorted(DEFAULT_TARGETS, key=lambda t: t.runOrder)): | ||
| if (target.name not in lhs_summary) or (target.name not in rhs_summary): | ||
|
|
||
| err = 1 | ||
|
|
||
| if target.name not in lhs_summary: | ||
| cons.print(f"{target.name} not present in lhs_summary - Case: {slug}") | ||
|
|
||
| if target.name not in rhs_summary: | ||
| cons.print(f"{target.name} not present in rhs_summary - Case: {slug}") | ||
|
|
||
| continue | ||
| cons.print(f"{target.name} not present in lhs_summary or rhs_summary - Case: {slug}") | ||
| err = 1; continue | ||
|
|
||
| if not math.isfinite(lhs_summary[target.name]["exec"]) or not math.isfinite(rhs_summary[target.name]["exec"]): | ||
| err = 1 | ||
| cons.print(f"lhs_summary or rhs_summary reports non-real exec time for {target.name} - Case: {slug}") | ||
|
|
||
| exec_time_speedup = "N/A" | ||
| try: | ||
| exec_time_speedup = f'{lhs_summary[target.name]["exec"] / rhs_summary[target.name]["exec"]:.2f}' | ||
| exec_time_value = lhs_summary[target.name]["exec"] / rhs_summary[target.name]["exec"] | ||
| if exec_time_value < 0.9: | ||
| cons.print(f"[bold yellow]Warning[/bold yellow]: Exec time speedup for {target.name} is less than 0.9 - Case: {slug}") | ||
| speedups[i] = f"Exec: {exec_time_value:.2f}" | ||
| if target == SIMULATION: | ||
| if not math.isfinite(lhs_summary[target.name]["grind"]) or not math.isfinite(rhs_summary[target.name]["grind"]): | ||
| err = 1 | ||
| cons.print(f"lhs_summary or rhs_summary reports non-real grind time for {target.name} - Case: {slug}") | ||
|
|
||
| grind_time_value = lhs_summary[target.name]["grind"] / rhs_summary[target.name]["grind"] | ||
| speedups[i] += f" & Grind: {grind_time_value:.2f}" | ||
| if grind_time_value <0.98: | ||
|
||
| raise MFCException(f"Benchmarking failed since grind time speedup for {target.name} below acceptable threshold (<0.98) - Case: {slug}") | ||
| except Exception as _: | ||
| err = 1 | ||
| cons.print(f"lhs_summary or rhs_summary reports non-real exec time for {target.name} - Case: {slug}") | ||
|
|
||
| speedups[i] = f"Exec: {exec_time_speedup}" | ||
|
|
||
| if target == SIMULATION: | ||
| grind_time_speedup = "N/A" | ||
| if not math.isfinite(lhs_summary[target.name]["grind"]) or not math.isfinite(rhs_summary[target.name]["grind"]): | ||
| err = 1 | ||
| cons.print(f"lhs_summary or rhs_summary reports non-real grind time for {target.name} - Case: {slug}") | ||
|
|
||
| try: | ||
| grind_time_speedup = f'{lhs_summary[target.name]["grind"] / rhs_summary[target.name]["grind"]:.2f}' | ||
| except Exception as _: | ||
| err = 1 | ||
| cons.print(f"lhs_summary or rhs_summary reports non-real grind time for {target.name} - Case: {slug}") | ||
|
|
||
| speedups[i] += f" & Grind: {grind_time_speedup}" | ||
| cons.print(f"lhs_summary or rhs_summary reports non-real grind time for {target.name} - Case: {slug}") | ||
|
|
||
| table.add_row(f"[magenta]{slug}[/magenta]", *speedups) | ||
|
|
||
| cons.raw.print(table) | ||
|
|
||
| if err != 0: | ||
| if err: | ||
| raise MFCException("Benchmarking failed") | ||
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.
I removed case optimization and capped gbpp to only 0.5.
This should slow all cases down drastically. I am not sure but It might exceed the allocated time. If happens to be the case, I will hop into Phoenix and figure out proper time for the bench job until I reach grind time failure mode.
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.
Currently testing the failure mode on Delta and will post the outcomes here
I guess I will cap just the memory per process and keep case optimization to make it run slower but do not take forever
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.
I overthought over it needlessly. I can just induce an artificial failure by modifying the grind times in the pr/master benchmark yaml files locally then run
bench_diff.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.
it worked out luckily