Channels await fix

[jwood803]

Fix for #5312

Setting as a draft to add a test.

[codecov]

Codecov Report

Merging #5313 into master will increase coverage by 13.87%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master    #5313       +/-   ##
===========================================
+ Coverage   73.80%   87.68%   +13.87%     
===========================================
  Files        1022      244      -778     
  Lines      190617    44759   -145858     
  Branches    20488     1900    -18588     
===========================================
- Hits       140686    39247   -101439     
+ Misses      44411     5167    -39244     
+ Partials     5520      345     -5175     

Flag	Coverage Δ
#Debug	87.68% <100.00%> (+13.87%)	⬆️
#production	?
#test	87.68% <100.00%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...est/Microsoft.ML.Tests/Scenarios/RegressionTest.cs	100.00% <100.00%> (ø)
test/Microsoft.ML.Functional.Tests/Validation.cs	100.00% <0.00%> (ø)
....CodeGenerator.Tests/ApprovalTests/TemplateTest.cs	100.00% <0.00%> (ø)
...r.Tests/ApprovalTests/ConsoleCodeGeneratorTests.cs	100.00% <0.00%> (ø)
src/Microsoft.ML.Transforms/Expression/Node.cs
src/Microsoft.ML.Core/Data/KeyTypeExtensions.cs
...icrosoft.ML.Data/EntryPoints/PredictorModelImpl.cs
...utoML/TrainerExtensions/TrainerExtensionCatalog.cs
src/Microsoft.ML.Data/EntryPoints/InputBuilder.cs
...ble/Selector/FeatureSelector/AllFeatureSelector.cs
... and 774 more

[jwood803]

Marked as ready for review, though still looking at how to add a test for this.

[mstfbl]

Can you add a comment here stating that the check to verify the operation is complete is necessary before calling GetResult()?

[max-test5]

Dear sir, I have same error with :

        var option = new SdcaMaximumEntropyMulticlassTrainer.Options()
        {
            FeatureColumnName = "SearchModel",
            LabelColumnName = "Label",
            NumberOfThreads = 1
        };
        var maximumEntropy = ctx.MulticlassClassification.Trainers.SdcaMaximumEntropy(option);
        var pipeline =ctx.Transforms.Conversion.MapValueToKey("Label", "BestDictMatchModelText") 
                .Append(ctx.Transforms.Text.FeaturizeText(inputColumnName: "VEHICLE_MODEL",
                    outputColumnName: "SearchModel"))
                .Append(maximumEntropy)
                .Append(ctx.Transforms.Conversion.MapKeyToValue("PredictedLabel"));

        var part1 = decoratedWithFuzzyList;//.Take(1000);

        var dataReader = ctx.Data.LoadFromEnumerable(part1);
        ThisModel = pipeline.Fit(dataReader);

---

So, when part1 contains more than a 1000 items, Fit throws:

at System.Threading.Channels.AsyncOperation.ThrowIncompleteOperationException()
at System.Threading.Channels.AsyncOperation1.GetResult(Int16 token) at Microsoft.ML.Transforms.RowShufflingTransformer.Cursor.MoveNextCore() at Microsoft.ML.Data.RootCursorBase.MoveNext() at Microsoft.ML.Trainers.TrainingCursorBase.MoveNext() at Microsoft.ML.Trainers.SdcaTrainerBase3.TrainCore(IChannel ch, RoleMappedData data, LinearModelParameters predictor, Int32 weightSetCount)
at Microsoft.ML.Trainers.StochasticTrainerBase2.TrainModelCore(TrainContext context) at Microsoft.ML.Trainers.TrainerEstimatorBase2.TrainTransformer(IDataView trainSet, IDataView validationSet, IPredictor initPredictor)
at Microsoft.ML.Trainers.TrainerEstimatorBase2.Fit(IDataView input) at Microsoft.ML.Data.EstimatorChain1.Fit(IDataView input)

Can you please consider to fix this?

[eNeRGy164]

I have this same issue with version 1.5.0. I can fit 1046 rows in the shuffled set. 1047 rows will give the The asynchronous operation has not completed exception.
Tested with the daily 1.5.2 package, and now the error is gone.

[eerhardt]

though still looking at how to add a test for this.

It sounds like the samples was able to repro the issue: dotnet/machinelearning-samples#833

[eerhardt]

This is undo-ing what we were trying to avoid with #5123 (comment), which is just spinning a thread seeing if there is something to read.

I was mistaken that you could block synchronously on a Channel. Sorry about that.

What do you think about going back to what you had previously with just calling TryRead, and if that returns false, the only thing I can think of is to Thread.Sleep for some amount of time, even 1 ms, just to stop the thread from consuming a whole core spinning waiting for the producer. It would probably be a good idea to try it multiple ways and see what perf results you get.

[jwood803]

Thanks @eerhardt! Made an update for this. Just let me know if it's still not correct.

[eerhardt]

Since this has broken projects in the past, it would be really good to get a test for this to make sure it doesn't break in the future.

[jwood803]

@eerhardt Added a small scenario based on the taxi fare sample. Verified in a separate solution that it throws the error and running the test in this branch runs successfully.

Definitely let me know if any changes are needed. Thanks for the help with this!

[eerhardt]

We already have this file in the repo. See

machinelearning/test/Microsoft.ML.Tests/FeatureContributionTests.cs

Line 417 in 37edde9

var trainDataPath = GetDataPath("taxi-fare-train.csv");

machinelearning/test/Microsoft.ML.Tests/FeatureContributionTests.cs

Lines 388 to 417 in 37edde9

	public class TaxiTrip
	{
	[LoadColumn(0)]
	public string VendorId;
	[LoadColumn(1)]
	public float RateCode;
	[LoadColumn(2)]
	public float PassengerCount;
	[LoadColumn(3)]
	public float TripTime;
	[LoadColumn(4)]
	public float TripDistance;
	[LoadColumn(5)]
	public string PaymentType;
	[LoadColumn(6)]
	public float FareAmount;
	}
	/// <summary>
	/// Returns a DataView with a Features column which include HotEncodedData
	/// </summary>
	private IDataView GetOneHotEncodedData(int numberOfInstances = 100)
	{
	var trainDataPath = GetDataPath("taxi-fare-train.csv");

We shouldn't need to download the file again.

[eerhardt]

All code files should have a copyright at the top.

[eerhardt]

LGTM. Thanks for fixing this @jwood803.

[jwood803]

Thanks again for the help @eerhardt! Sorry for breaking it, but glad I can help with the fix.