Add metric structure - #109
Conversation
Codecov Report❌ Patch coverage is
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
BatyLeo
left a comment
There was a problem hiding this comment.
Thank you for the PR, that's a good first draft. I put some first comments, I think several things can be improved, but we need to first converge on the Policy PRs in order to see what's the best way to manage metrics.
| """ | ||
| $TYPEDEF | ||
|
|
||
| Abstract type for evaluation metrics, used to evaluate and compare [`Policy`](@ref)s |
There was a problem hiding this comment.
suggestion: we probably can compare any AbstractPolicy, not only those of type Policy. This comment can be updated once the other PR on policies is merged
| on a benchmark. Parametrized by the benchmark type `B` the metric is bound to, | ||
| this makes sense since an evaluation process is always benchmark-specific. | ||
|
|
||
| The metrics logic is splitted between static and dynamic benchmark: |
There was a problem hiding this comment.
question: what about stochastic benchmarks, are they considered as static in this case?
There was a problem hiding this comment.
Indeed this feels weird... can we improve the naming ? Should we consider 3 metric types...?
There was a problem hiding this comment.
I added a comment on this in commit 10617f7.
indeed, for now, the package handles the evaluation of stochastic benchmarks via SampleAverageApproximation which is an AbstractStaticBenchmark ==> we handle their metric through AbstractStaticMetric.
I would be glad to help with a native implementation for evaluating stochastic benchmark (with a one step rollout ?)
As described in issue #107, the package lacks a way to handle metrics which would be beneficial to users who want to compare the performances of several policies on their benchmark.
What this adds
The main file added is
src/Utils/metric.jlThe main method is the
AbstractMetric{B<:AbstractBenchmark}: every metric is bound to a benchmark instanceA nice sub structure is also define :
Metric{B,F}=> takes as input a callable metric wrapping any (bench, dataset) -> Real for quick user-defined metrics without writing a dedicated type.It also implements several
evaluate_metricthat can handle a single dataset, multiple datasets (returns the vector of values) or multiple pairs ("policy", datasets), which returns aVector{MetricStats}for one-call multi-policy evaluation.MetricStats: per-dataset values with mean_metric/std_metric/quantile_metric functions. It carries a label naming the policy that produced it (e.g. a Policy's name).We also implement two predefined metrics: reward and gap.
RewardMetric(bench; op=mean)- works for every benchmark:objective_valuefor staticsample.extra.rewardfor dynamicRelativeGapMetric(bench, statistical_model, maximizer; op=mean)- wraps the existing compute_gap as a metricIt also adds plotting functions : boxplots for metric comparisons
Notes / follow-ups
evaluate_policiesfunction could be nice to have to be able to evaluate several stochastic benchmarks on multiple policies.