ML.NET public API exposes parameter weights which is not used.

[abgoswam]

For some of the learners e.g. SdcaRegression ML.NET public API exposes parameter weights

machinelearning/src/Microsoft.ML.StandardLearners/StandardLearnersCatalog.cs

Lines 65 to 70 in cabf55b

	public static SdcaRegressionTrainer StochasticDualCoordinateAscent(this RegressionContext.RegressionTrainers ctx,
	string labelColumn = DefaultColumnNames.Label,
	string featureColumn = DefaultColumnNames.Features,
	string weights = null,
	ISupportSdcaRegressionLoss loss = null,
	float? l2Const = null,

However, the advanced Options for SdcaRegressionTrainer do not have a field for 'WeightColumn'. Also, I believe the algo itself does not use weights (need to verify)

We need to scrub our public API for such spurious uses of weights parameter.

EDIT :

• Based on investigations (see below), we see that the SdcaRegressionTrainer does use the weights column. So the proper fix would be to ensure that the weight column passed in the constructor gets used by the trainer.
• For the example above, we need to add WeightColumn in the advanced Options for SdcaRegressionTrainer and verify it gets used by the SDCA trainer.

@sfilipi @glebuk @TomFinley

[glebuk]

There are several additional problems with weigh parameters::

• Name of the argument should be consistent with outer column names, it should end on *Column.
• method should only have the parameter when algo uses it.
• the term "weight" is ambigious with model weights. Perhaps we should disambiguate this further by calling it exampleWeightColumn
• Confusing behavior between providing input values null and "weight":

1. if arg is not specified and Weight column exists - column is used
2. if arg is not specified and Weight column does NOT exist - no error
3. if arg is specified and Weight column exist - column is used
4. if arg is specified and Weight column does NOT exist - crash!

This comes from the command line days. However it does not make much sense from the API point of veiw.

[glebuk]

seems related to issue #2177

[TomFinley]

However, the advanced Options for SdcaRegressionTrainer do not have a field for 'WeightColumn'. Also, I believe the algo itself does not use weights (need to verify)

Hi @abgoswam ... from what I see, it does. See here, in the SdcaTrainerBase.TrainCore method:

machinelearning/src/Microsoft.ML.StandardLearners/Standard/SdcaBinary.cs

Line 556 in 3e03fce

invariants[idx] = Loss.ComputeDualUpdateInvariant(invariantCoeff * normSquared * lambdaNInv * GetInstanceWeight(cursor));

If the "connection" from weights in the constructor to weights in the trainer has been broken, that is what must be fixed. You should not remove the option -- you should make it work, right? At least so I would think.

[abgoswam]

@TomFinley . Correct. As you pointed out, for this issue we need a fix whereby the weight column passed in the constructor gets used by the trainer.

I have edited the bug description accordingly.

[abgoswam]

@glebuk . I created a separate issue #2257 for the 2 comments above regarding naming convention for the weights parameter

• Name of the argument should be consistent with outer column names, it should end on *Column.
• The term "weight" is ambigious with model weights. Perhaps we should disambiguate this further by calling it exampleWeightColumn

[abgoswam]

Removing api label, added bug label. Hence removing it from Project13 as well.