Modified cut_optimiser tool #1188
Conversation
…ps over repeated sets of parameters
…ps over repeated sets of parameters
…the same file. Included CLI option option for parameter deltaZMax
| @@ -128,7 +134,8 @@ | |||
| m_seedfinder.impactMax *= unit<float>::mm; | |||
| m_seedfinder.maxPtScattering *= unit<float>::GeV; | |||
| m_seedfinder.bFieldInZ *= unit<float>::T; | |||
|
|
|||
| m_seedfinder.deltaZMax *= unit<float>::mm; | |||
|
|
|||
There was a problem hiding this comment.
Please put your C++ changes in their own, separate PR.
| "seedfinder-deltaZMax", | ||
| po::value(&m_seedfinder.deltaZMax) | ||
| ->default_value(m_seedfinder.impactMax / unit<float>::mm), | ||
| "Maximum deltaZ parameter [mm]"); |
There was a problem hiding this comment.
| "Maximum deltaZ parameter [mm]"); | |
| "Maximum Z distance between measurements [mm]"); |
| "Maximum impact parameter", | ||
| std::format("{:.2f} mm", m_seedfinder.impactMax / unit<float>::mm))); | ||
| cat->add_child(std::make_unique<configuration_kv_pair>( | ||
| "Maximum deltaZ parameter", |
There was a problem hiding this comment.
| "Maximum deltaZ parameter", | |
| "Maximum Z distance between measurements", |
| log.info("Design space has been exhausted; exiting") | ||
| break | ||
| if param_list in results: | ||
| if param_list in parameters: |
There was a problem hiding this comment.
Why are you not using results here?
There was a problem hiding this comment.
results doesn't contain the parameter values, it just contain efficiencies and fake rates. I could have used key to access parameters, but param_list is a list of floating point numbers hence comparing against another list.
| #except subprocess.CalledProcessError as e: | ||
| # log.warning("Process failed to execute; continuing") | ||
| # results[param_list] = None | ||
| # continue |
extras/cut_optimiser/main.py
Outdated
| result_efficiency * 100.0, | ||
| result_duplicate_rate, | ||
| result_fake_rate, | ||
| "Physics performance was %(efficiency).1f%% efficiency, %(fake).1f fake rate, %(duplicate).1f duplicate rate, %(seed_efficiency).1f%% seeding efficiency, %(seed_fake).1f seed fake rate, %(seed_duplicate).1f seed duplicate rate", |
There was a problem hiding this comment.
Please use "seed" and "seeding" consistently.
| seeding_efficiency=0.0, | ||
| seed_fake_rate=0.0, | ||
| seed_duplicate_rate=0.0, |
There was a problem hiding this comment.
Don't you need to update the Pareto front finder if you make this change?
There was a problem hiding this comment.
changed in find_pareto_set.py
| example_ACTS.json | ||
| example_ACTS.json~ | ||
| example.json~ | ||
| main_test.py | ||
| main_test.py~ | ||
| seeding_param.json | ||
| seeding_param.json~ |
There was a problem hiding this comment.
| example_ACTS.json | |
| example_ACTS.json~ | |
| example.json~ | |
| main_test.py | |
| main_test.py~ | |
| seeding_param.json | |
| seeding_param.json~ | |
| *~ |
There was a problem hiding this comment.
This is not really what I meant; you can remove all the other excludes if you just add the one I suggested.
| m_desc.add_options()( | ||
| "seedfinder-deltaZMax", | ||
| po::value(&m_seedfinder.deltaZMax) | ||
| ->default_value(m_seedfinder.impactMax / unit<float>::mm), |
There was a problem hiding this comment.
This default value is incorrect.
There was a problem hiding this comment.
Sorry about that. This changes to track_seeding.cpp is addressed in the new PR
| 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"] |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Yes that might be better. Maybe Adding two CLI options to seeding performance and finding performance?
There was a problem hiding this comment.
Yes I think that's a good idea!
|
There was a problem hiding this comment.
C++ changes should now be removed from this PR.
2 participants
The cut_optimiser tool has been modified to include the feature that it will skip the benchmark step over sets of parameter values it already ran over already and if it exists in the .csv file.