-
Notifications
You must be signed in to change notification settings - Fork 58
Modified cut_optimiser tool #1188
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
base: main
Are you sure you want to change the base?
Changes from all commits
1187164
22e19c9
9fd3383
9f1f2cf
af028cb
8926037
74757ca
3d9204f
644b1b2
1393111
0d6104c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
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. C++ changes should now be removed from this PR. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,10 @@ def main(): | |
| and n["efficiency"] >= m["efficiency"] | ||
| and n["fake_rate"] <= m["fake_rate"] | ||
| and n["duplicate_rate"] <= m["duplicate_rate"] | ||
| and n["seeding_efficiency"] >= m["seeding_efficiency"] | ||
| and n["seed_fake_rate"] <= m["seed_fake_rate"] | ||
| and n["seed_duplicate_rate"] <= m["seed_duplicate_rate"] | ||
|
Comment on lines
+62
to
+64
Member
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. Keep in mind that the Pareto set of the seeding parameters and the finding parameters is much larger than the Pareto set of just the seeding parameters or the finding parameters. Would it make sense to add a CLI switch that can be used to select the user's preference?
Author
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. Yes that might be better. Maybe Adding two CLI options to seeding performance and finding performance?
Member
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. Yes I think that's a good idea! |
||
|
|
||
| ): | ||
| log.debug( | ||
| "Removing %s from the Pareto set because %s is superior", | ||
|
|
@@ -73,11 +77,14 @@ def main(): | |
|
|
||
| for i in sorted(pareto_set, key=lambda x: x["rec_throughput"], reverse=True): | ||
| log.info( | ||
| " Eff. %.2f, fake rate %.2f, duplicate rate %.2f with reciprocal througput %.1fms is achieved by setup {%s}", | ||
| " Eff. %.2f, fake rate %.2f, duplicate rate %.2f with reciprocal througput %.1fms and Seeding Eff. %.2f, Seed fake rate %.2f, Seed duplicate rate %.2f is achieved by setup {%s}", | ||
| 100.0 * i["efficiency"], | ||
| i["fake_rate"], | ||
| i["duplicate_rate"], | ||
| i["rec_throughput"] * 1000.0, | ||
| i["seeding_efficiency"], | ||
| i["seed_fake_rate"], | ||
| i["seed_duplicate_rate"], | ||
| ", ".join( | ||
| "%s: %s" % (k, str(v)) | ||
| for k, v in i.items() | ||
|
|
@@ -87,6 +94,9 @@ def main(): | |
| "fake_rate", | ||
| "duplicate_rate", | ||
| "rec_throughput", | ||
| "seeding_efficiency", | ||
| "seed_fake_rate", | ||
| "seed_duplicate_rate", | ||
| "success", | ||
| ] | ||
| ), | ||
|
|
||
|
Member
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. This file needs to be formatted with Black.
Author
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 didn't get this part. What do you mean formatted with black?
Member
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. Black is a Python formatter. You can run it by executing |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,15 +14,14 @@ | |
| import subprocess | ||
| import re | ||
|
|
||
|
|
||
| import traccc_bench_tools.parse_profile | ||
| import traccc_bench_tools.types | ||
|
|
||
|
|
||
| log = logging.getLogger("traccc_cut_optimiser") | ||
|
|
||
|
|
||
| def main(): | ||
| def main(): | ||
| parser = argparse.ArgumentParser() | ||
|
|
||
| parser.add_argument( | ||
|
|
@@ -108,7 +107,7 @@ def main(): | |
| params = json.load(f) | ||
|
|
||
| parameter_space = params["parameters"] | ||
| parameter_names = sorted(parameter_space) | ||
| parameter_names = list(parameter_space) | ||
|
|
||
| log.info( | ||
| "Running optimisation for %d parameters: %s", | ||
|
|
@@ -122,29 +121,39 @@ def main(): | |
| "efficiency", | ||
| "fake_rate", | ||
| "duplicate_rate", | ||
| "seeding_efficiency", | ||
| "seed_fake_rate", | ||
| "seed_duplicate_rate", | ||
| ] | ||
|
|
||
| parameters = [] | ||
|
Member
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 believe you already fixed this bug and you should now be using
Author
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. No I need parameters. I am using results and parameters as separate lists
Member
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. But isn't |
||
| if args.db.is_file(): | ||
| log.info('Database file "%s" already exists; creating a backup', args.db) | ||
| shutil.copy(str(args.db), str(args.db) + ".bak") | ||
| results = {} | ||
| with open(args.db, "r") as f: | ||
| reader = csv.DictReader(f) | ||
| for i in reader: | ||
| key = (i[k] for k in parameter_names) | ||
| key = tuple(i[k] for k in parameter_names) | ||
| pkey = tuple(float(i[k]) for k in parameter_names) | ||
| parameters.append(pkey) | ||
| if set(i.keys()) != set(csv_field_names): | ||
| raise ValueError( | ||
| "Input database has unexpected columns or is missing columns" | ||
| ) | ||
| if i["success"] != 0: | ||
| set_skip = False | ||
| results[key] = { | ||
| "rec_throughput": i["rec_throughput"], | ||
| "efficiency": i["efficiency"], | ||
| "fake_rate": i["fake_rate"], | ||
| "duplicate_rate": i["duplicate_rate"], | ||
| "seeding_efficiency": i["seeding_efficiency"], | ||
| "seed_fake_rate": i["seed_fake_rate"], | ||
| "seed_duplicate_rate": i["seed_duplicate_rate"], | ||
| } | ||
| else: | ||
| results[key] = None | ||
| results[key] = None | ||
| log.info("Database contained %d pre-existing results", len(results)) | ||
| else: | ||
| log.info('Database file "%s" does not exist; starting from scratch', args.db) | ||
|
|
@@ -178,8 +187,7 @@ def main(): | |
| if args.random: | ||
| param_dict = {n: random.choice(vs) for n, vs in parameter_space.items()} | ||
| param_list = tuple(param_dict.values()) | ||
|
|
||
| if param_list in results: | ||
| if param_list in parameters: | ||
| random_retry_count += 1 | ||
| if random_retry_count < 10: | ||
| log.info( | ||
|
|
@@ -203,7 +211,7 @@ def main(): | |
| except StopIteration: | ||
| log.info("Design space has been exhausted; exiting") | ||
| break | ||
| if param_list in results: | ||
| if param_list in parameters: | ||
|
Member
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. Why are you not using results here?
Author
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. results doesn't contain the parameter values, it just contain efficiencies and fake rates. I could have used key to access parameters, but |
||
| log.info( | ||
| "Configuration %s is already known; continuing", str(param_dict) | ||
| ) | ||
|
|
@@ -241,8 +249,7 @@ def main(): | |
| "--grid-file=%s" % params["input"]["grid_file"], | ||
| "--material-file=%s" % params["input"]["material_file"], | ||
| "--input-events=1", | ||
| "--check-performance", | ||
| "--use-acts-geom-source=on", | ||
| "--check-performance", | ||
| ] | ||
|
|
||
| for k, v in params["config"].items(): | ||
|
|
@@ -253,27 +260,26 @@ def main(): | |
|
|
||
| if args.ncu_wrapper is not None: | ||
| profile_args = args.ncu_wrapper.split() + profile_args | ||
|
|
||
| try: | ||
| result = subprocess.run( | ||
| profile_args, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.STDOUT, | ||
| cwd=tmppath, | ||
| check=True, | ||
| timeout=args.timeout, | ||
| profile_args, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.STDOUT, | ||
| cwd=tmppath, | ||
| check=True, | ||
| timeout=args.timeout, | ||
| ) | ||
| except subprocess.CalledProcessError as e: | ||
| log.warning("Process failed to execute; continuing") | ||
| results[param_list] = None | ||
| continue | ||
| #except subprocess.CalledProcessError as e: | ||
| # log.warning("Process failed to execute; continuing") | ||
| # results[param_list] = None | ||
| # continue | ||
|
Comment on lines
+273
to
+276
Member
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. Why did you remove this?
Author
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. un-commented now
Member
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. It's still commented. |
||
| except subprocess.TimeoutExpired as e: | ||
| log.warning("Process timed out; marking as failure and continuing") | ||
| results[param_list] = None | ||
| continue | ||
|
|
||
| stdout = result.stdout.decode("utf-8") | ||
|
|
||
| if match := re.search( | ||
| r"Total track efficiency was (\d+(?:\.\d*)?)%", stdout | ||
| ): | ||
|
|
@@ -294,12 +300,39 @@ def main(): | |
| result_fake_rate = float(match.group(1)) | ||
| else: | ||
| raise ValueError("Fake rate could not be parsed from stdout!") | ||
|
|
||
| if match := re.search( | ||
| r"Total seed efficiency was (\d+(?:\.\d*)?)%", stdout | ||
| ): | ||
| result_seeding_efficiency = float(match.group(1)) / 100.0 | ||
| else: | ||
| raise ValueError("Seeding Efficiency could not be parsed from stdout!") | ||
|
|
||
| if match := re.search( | ||
| r"Total seed fake rate was (\d+(?:\.\d*)?)", stdout | ||
| ): | ||
| result_seed_fake_rate = float(match.group(1)) | ||
| else: | ||
| raise ValueError("Seed fake rate could not be parsed from stdout!") | ||
|
|
||
| if match := re.search( | ||
| r"Total seed duplicate rate was (\d+(?:\.\d*)?)", stdout | ||
| ): | ||
| result_seed_duplicate_rate = float(match.group(1)) | ||
| else: | ||
| raise ValueError("Seed Duplicate rate could not be parsed from stdout!") | ||
|
|
||
|
|
||
| log.info( | ||
| "Physics performance was %.1f%% efficiency, %.1f fake rate, %.1f duplicate rate", | ||
| result_efficiency * 100.0, | ||
| result_duplicate_rate, | ||
| result_fake_rate, | ||
| "Physics performance was %(efficiency).1f%% efficiency, %(fake).1f fake rate, %(duplicate).1f duplicate rate, %(seeding_efficiency).1f%% seeding efficiency, %(seed_fake).1f seed fake rate, %(seed_duplicate).1f seed duplicate rate", | ||
| { | ||
| "efficiency": result_efficiency * 100.0, | ||
| "fake": result_fake_rate, | ||
| "duplicate": result_duplicate_rate, | ||
| "seeding_efficiency": result_seeding_efficiency * 100.0, | ||
| "seed_fake": result_seed_fake_rate, | ||
| "seed_duplicate": result_seed_duplicate_rate, | ||
| }, | ||
| ) | ||
|
|
||
| end_time = time.time() | ||
|
|
@@ -337,6 +370,9 @@ def main(): | |
| "efficiency": result_efficiency, | ||
| "fake_rate": result_fake_rate, | ||
| "duplicate_rate": result_duplicate_rate, | ||
| "seeding_efficiency": result_seeding_efficiency, | ||
| "seed_fake_rate": result_seed_fake_rate, | ||
| "seed_duplicate_rate": result_seed_duplicate_rate, | ||
| } | ||
|
|
||
| except Exception as e: | ||
|
|
@@ -348,16 +384,16 @@ def main(): | |
|
|
||
| log.info("Gathered a total of %d results (incl. pre-existing)", len(results)) | ||
|
|
||
|
|
||
| log.info("Writing data to %s", args.db) | ||
| with open(args.db, "w") as f: | ||
| writer = csv.DictWriter( | ||
| f, | ||
| fieldnames=csv_field_names, | ||
| ) | ||
| writer.writeheader() | ||
|
|
||
| for k, v in results.items(): | ||
|
|
||
| param_dict = {parameter_names[i]: p for i, p in enumerate(k)} | ||
| if v is None: | ||
| writer.writerow( | ||
|
|
@@ -368,6 +404,9 @@ def main(): | |
| efficiency=0.0, | ||
| fake_rate=0.0, | ||
| duplicate_rate=0.0, | ||
| seeding_efficiency=0.0, | ||
| seed_fake_rate=0.0, | ||
| seed_duplicate_rate=0.0, | ||
|
Comment on lines
+407
to
+409
Member
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. Don't you need to update the Pareto front finder if you make this change?
Author
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. changed in |
||
| ) | ||
| ) | ||
| else: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
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.
Thanks. Added
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 is not really what I meant; you can remove all the other excludes if you just add the one I suggested.