Summary
ml.Metric is exported and every method it requires is exported, so a caller can implement it outside the package. But the routing that decides what a metric receives keys off two unexported marker interfaces, which a caller cannot implement:
https://github.com/HazelnutParadise/insyra/blob/main/ml/model_selection.go#L401-L407
type classLabelMetric interface {
needsClassLabels() bool
}
type probabilityMetric interface {
needsProbabilities() bool
}
predictionForMetric (ml/model_selection.go:627) tests for these to decide whether to hand the metric probabilities, class labels, or raw values. An external metric matches neither, so it always falls through to model.Predict.
What this costs
A metric that needs class labels works, because Classifier.Predict returns labels. A metric that needs probabilities cannot be written outside the package at all — there is no way to ask for them.
That rules out custom Brier score, custom calibration measures, custom multi-class log loss variants, and any probability-based business metric — the exact cases where a caller writes their own metric rather than using a built-in one.
Demonstrated:
type externalMetric struct{ got ml.Prediction }
func (*externalMetric) Name() string { return "external" }
func (*externalMetric) Kind() ml.MetricKind { return ml.ClassificationMetric }
func (m *externalMetric) Evaluate(y *insyra.DataList, p ml.Prediction) (ml.MetricResult, error) {
m.got = p
return ml.MetricResult{Name: "external", Score: 0}, nil
}
// CrossValidate with a logistic model, which IS a ProbaModel:
// external metric received: Values=true Probabilities=false Classes=true
The model supports probabilities, the harness knows how to fetch them, and the metric cannot say it wants them.
Why it is easy to miss
Nothing errors. The metric receives a well-formed Prediction with Values and Classes populated and Probabilities nil, so a caller who does not check for nil computes a score from the wrong input and gets a number back.
This is the same shape as two defects already fixed in this package — an interface satisfied in form while the substance is unreachable. SimpleImputer claimed InverseTransform it could not honour; LogisticModel.Predict returned probabilities where a Classifier promises labels. Both were caught. This one is the mirror image: the capability exists and the caller cannot reach it.
Possible directions
Not proposing one — flagging that the extension point does not extend.
- Export the markers.
ml.NeedsProbabilities / ml.NeedsClassLabels as exported one-method interfaces. Smallest change, keeps the existing dispatch.
- Put it on
MetricKind. A metric already declares its kind; add a value or a companion method saying what input it wants. Fewer interfaces, but changes an existing exported type.
- Hand every metric everything. Populate
Values, Probabilities and Classes whenever the model can supply them, and let the metric use what it needs. Simplest contract; costs a PredictProba call the metric may not want.
- Close the extension point. Unexport
Metric and ship only built-in metrics. Honest, but gives up custom metrics entirely.
Whichever is chosen, Prediction's field documentation should say which fields are populated when, since a nil Probabilities is currently indistinguishable from a model that has none.
Notes
- Found while verifying the
ml package against its OpenSpec changes. Not a wrong answer for any built-in metric — those all implement the unexported markers and route correctly.
- The built-in path is unaffected either way, so this is additive whichever direction is taken.
Summary
ml.Metricis exported and every method it requires is exported, so a caller can implement it outside the package. But the routing that decides what a metric receives keys off two unexported marker interfaces, which a caller cannot implement:https://github.com/HazelnutParadise/insyra/blob/main/ml/model_selection.go#L401-L407
predictionForMetric(ml/model_selection.go:627) tests for these to decide whether to hand the metric probabilities, class labels, or raw values. An external metric matches neither, so it always falls through tomodel.Predict.What this costs
A metric that needs class labels works, because
Classifier.Predictreturns labels. A metric that needs probabilities cannot be written outside the package at all — there is no way to ask for them.That rules out custom Brier score, custom calibration measures, custom multi-class log loss variants, and any probability-based business metric — the exact cases where a caller writes their own metric rather than using a built-in one.
Demonstrated:
The model supports probabilities, the harness knows how to fetch them, and the metric cannot say it wants them.
Why it is easy to miss
Nothing errors. The metric receives a well-formed
PredictionwithValuesandClassespopulated andProbabilitiesnil, so a caller who does not check for nil computes a score from the wrong input and gets a number back.This is the same shape as two defects already fixed in this package — an interface satisfied in form while the substance is unreachable.
SimpleImputerclaimedInverseTransformit could not honour;LogisticModel.Predictreturned probabilities where aClassifierpromises labels. Both were caught. This one is the mirror image: the capability exists and the caller cannot reach it.Possible directions
Not proposing one — flagging that the extension point does not extend.
ml.NeedsProbabilities/ml.NeedsClassLabelsas exported one-method interfaces. Smallest change, keeps the existing dispatch.MetricKind. A metric already declares its kind; add a value or a companion method saying what input it wants. Fewer interfaces, but changes an existing exported type.Values,ProbabilitiesandClasseswhenever the model can supply them, and let the metric use what it needs. Simplest contract; costs aPredictProbacall the metric may not want.Metricand ship only built-in metrics. Honest, but gives up custom metrics entirely.Whichever is chosen,
Prediction's field documentation should say which fields are populated when, since a nilProbabilitiesis currently indistinguishable from a model that has none.Notes
mlpackage against its OpenSpec changes. Not a wrong answer for any built-in metric — those all implement the unexported markers and route correctly.