Chronos: add independent chronos.metric
#3608
Conversation
| @@ -0,0 +1,59 @@ | |||
| # | |||
There was a problem hiding this comment.
good catch, I will fix it.
| Ground truth (correct) target values. | ||
| :param y_pred: Array-like of shape = (n_samples, \*). | ||
| Estimated target values. | ||
| :param multioutput: String in ['raw_values', 'uniform_average'] |
There was a problem hiding this comment.
"raw_values" and “uniform_average” is too long and does not accurately describe what it means ("raw_values" actually means "element-wise", and "uniform_average" actually means aggregated by mean (comparing againse element-wise). Actually the hyper parameter "multioutput“ itself is confusing as well.
My suggestion is: If we have only two options here, we can use a bool parameter, e.g. aggrerate=True or element-wise=True. If we have more than two aggregation option plus an element-wise option, then we can use aggregate='None', 'mean', etc.
There was a problem hiding this comment.
This setting is inherited from orca.automl (which inherited from sklearn). I think a string type aggregate is much better since we do plan to add more settings for our users' convenience.
| A floating point value, or an | ||
| array of floating point values, one for each individual target. | ||
| """ | ||
| metric = metric.lower() |
There was a problem hiding this comment.
evaluate does not take a list of metrics?
There was a problem hiding this comment.
This setting is inherited from orca.automl, I think we can support it easily.
| array of floating point values, one for each individual target. | ||
| """ | ||
| metric = metric.lower() | ||
| assert metric in TORCHMETRICS_REGRESSION_MAP.keys(),\ |
There was a problem hiding this comment.
should we use assert for parameter checks?
There was a problem hiding this comment.
I think it's OK? Since a false input will definitely give a error. Or should we use a NotImplementedError or RuntimeError here?
TheaperDeng
force-pushed
the
chronos-built-in-metrics
branch
from
e935df9 to
1a00a4e
Compare
TheaperDeng
force-pushed
the
chronos-built-in-metrics
branch
from
1a00a4e to
8220016
Compare
|
I will first merge this PR and add more uts and give some enhancement to |
2 participants
Why we need
chronos.metric?We need to complete
*.evaluate(...)in chronos with self-contained code without any dependency onbigdl-orca.Convenient for future light HPO.
Convenient for future AD HPO.
What does this PR do?
first version with basic test and similar API to orca, we may discuss about the details later.