-
Notifications
You must be signed in to change notification settings - Fork 203
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
Add levels to plot benchmarks #3765
base: main
Are you sure you want to change the base?
Add levels to plot benchmarks #3765
Conversation
…add-levels-to-plot-benchmarks
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.
Looks good to me, but I'll need to play a bit with it to understand exactly the levels. Each time a function in plot_tools will be added, it will need to handle case keys for grouping then, am I right?
def compute_gt_unit_locations(self, dataset_keys=None, **method_kwargs): | ||
if dataset_keys is None: | ||
dataset_keys = list(self.datasets.keys()) | ||
for dataset_key in dataset_keys: | ||
sorting_analyzer = self.get_sorting_analyzer(dataset_key=dataset_key) | ||
sorting_analyzer.compute("unit_locations", **method_kwargs) | ||
|
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.
Why a special focus on gt_unit_locations? I mean such a function could be great, to compute quantities on the main analyzer, but maybe it could be more generic and allow any extension to be computed? Or otherwise, let's discuss why only gt_unit_locations
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 because the the plot_performance_vs_depth_and_snr
uses it
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.
Changed in last commit to have a more general compute_analyzer_extension
and get_gt_unit_locations
(which uses the gt_unit_location
property if available)
@yger let me share some examples. You can check the complex study in the test, which has 8 datasets and 16 cases (tdc2 and sc2). Now, if you run the plotting functions without levels, you would get 16 cases, which is very messy... instead, let's say I'm only interested in the effect of Here's what you get: plot_run_timesplot_performances_orderedperformance VS SNRperformance VS SNR+Depthperformance comparison |
Nice, thanks for sharing the example, this is great. And can you provide a custom palette for your cases? Let say for example I want SC2 to only be orange and TDC2 to only be blue (with different alpha depending on cases). Is it doable ? I'll read it, but this seems very practical to ease the lecture of the graphs |
Let me add some customization there |
@yger with the last commit you can specify colors: Examples:
or:
If you specify wrong colors based on the levels, you'll get this:
Error:
|
This looks good to me. It might be out of the scope of this PR, but all these plotting functions in benchmarks are working for Study object. Would it be worth making them also work for single Comparisons? Not sure this is crucial, just a thought |
You mean for a single Benchmark? |
Yes, I am not using these syntax anymore and maybe this will be deprecated, but for a while we had functions such as plot_performances(comp), where comp was a single Benchmark (GTComparison for example ?). But i should read the code again, maybe this is now a Study and all these plots in widgets will still work |
Replacement for #3301
This adds the option to aggregate levels when plotting benchmark results.