# ML\.NET: can Microsoft's machine learning be trusted?

In 2018, Microsoft created ML\.NET, a machine learning framework for \.NET developers\. Since then, the machine learning library has undergone significant changes and acquired new features to identify patterns within data\. Let's see how these changes have affected the quality of ML\.NET source code\. And explore the errors, of course\!

![0988_ML_NET/image1.png](https://import.viva64.com/docx/blog/0988_ML_NET/image1.png)

## Introduction

Before we start, let's have a closer look at the framework\. Microsoft created ML\.NET as a free, open\-source machine learning library for the C\# and F\# programming languages\. ML\.NET allows you to build functionality for automated predictions using the dataset available to your application\.

You can use ML\.NET to train a custom machine learning model by specifying an algorithm, or you can import the pre\-trained TensorFlow and ONNX models\.

You can add the model to your application to get predictions\.

At the time of writing, ML\.NET included the following: classification/categorization, regression, anomaly detection, recommender systems implementation, image classification, time series and sequential data analysis\.

Knowing what ML\.NET is, we can now move on to inspect the errors PVS\-Studio found\. But first, we should give credit to Microsoft developers, since the project's code is of high quality: it was quite a challenge to find any terrible or curious errors for this article\. Yet, there are a few critical issues worthy of attention\. 

We checked the ML\.NET 1\.7\.1 version\. The source code of this project's version is available on [GitHub](https://github.com/dotnet/machinelearning/releases/tag/v1.7.1)\.

So, let's dive in and see what PVS\-Studio found in ML\.NET\!

## ML\.NET error review

### An object was used after a null check

**Issue 1**

```cpp
private EstimatorChain(...., IEstimator<ITransformer>[] estimators, ....)
{
  ....
  _estimators = estimators ?? new IEstimator<ITransformer>[0]; // <=
  ....
  LastEstimator = estimators.LastOrDefault() as // <=
                  IEstimator<TLastTransformer>;

  _needCacheAfter = needCacheAfter ?? new bool[0];
  ....
}
```

PVS\-Studio warning: [V3125](https://pvs-studio.com/en/docs/warnings/v3125/)\. The 'estimators' object was used after it was verified against null\. EstimatorChain\.cs 37, 39

Let's take a closer look at the warning\. PVS\-Studio detected a possible error that could lead to a null reference dereferencing\. Let's examine the code\. The null\-coalescing operator checks whether the _estimators_ input parameter carries a value\. The operator's result is recorded to the _\_estimators_ field\. 

On the line below, developers use the _estimators_ parameter once again, calling the _LastOrDefault method\. _Accessing the _estimators_ parameter that may have a null value may cause _NullReferenceException_\.

The solution to this problem is to use the _\_estimators_ field — instead of the _estimators_ variable — to call the _LastOrDefault_ method:

```cpp
LastEstimator = _estimators.LastOrDefault() as IEstimator<TLastTransformer>;
```

Names of the _estimators_ parameter and the _\_estimators_ class field are quite similar\. Developers may have just mixed them up\. When writing code, they could easily miss such a small detail\. But the static analyzer pays attention even to the smallest elements of code\. Thus, you do not have to search for these errors and correct them afterwards\.

### Two 'if' statements with identical conditional expressions

**Issue 2**

```cpp
private protected override void CheckOptions(IChannel ch)
{
  ....
  if (FastTreeTrainerOptions.NumberOfLeaves > 2 &&
      FastTreeTrainerOptions.HistogramPoolSize >
      FastTreeTrainerOptions.NumberOfLeaves - 1)
  {
    throw ch.Except("Histogram pool size (ps) must be at least 2."); 
  }

  if (FastTreeTrainerOptions.NumberOfLeaves > 2 &&
      FastTreeTrainerOptions.HistogramPoolSize >
      FastTreeTrainerOptions.NumberOfLeaves - 1)
  {
    throw ch.Except("Histogram pool size (ps) must be at most numLeaves - 1."); 
  }
  ....
}
```

PVS\-Studio warning: [V3021](https://pvs-studio.com/en/docs/warnings/v3021/) There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless\. BoostingFastTree\.cs 44, 47

The analyzer detected two similar _if_ statements with identical conditional expressions\. The program terminates since the _if_ block throws an exception\. It may just look like a useless pile of code, but let's inspect the problem from a different angle\. Look closer at the exceptions' error messages – they are not identical\. The developers probably intended to implement two different checks\. They may have copied the check, changed the message, but forgot to correct the condition\. Thus, one possible error remained unhandled and still poses a threat to the full and correct operation of the program\.

### The variable was used after it was assigned a value through null\-conditional operator

**Issue 3**

```cpp
private static int CheckKeyLabelColumnCore<T>(....) where T : ....
{
  ....
  for (int i = 1; i < models.Length; i++)
  {
    ....
    var mdType = labelCol.Annotations
                         .Schema
                         .GetColumnOrNull(....) // <=
     ?.Type;

    if (!mdType.Equals(keyValuesType))
      throw env.Except(....);
    ....
  }
  ....
}
```

PVS\-Studio warning: [V3105](https://pvs-studio.com/en/docs/warnings/v3105/) The 'mdType' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. PipelineEnsemble\.cs 670

This time, the analyzer issues warnings on a possible occurrence of _NullReferenceException_ when dereferencing a variable that may the _null_ value\. The analyzer thinks so because there is no null check between when the value is assigned and when it is used\. Look at the code\. The _mdType_ variable is assigned with the value through a call chain\. One of the parts of the chain is the _GetColumnOrNull_ method, which returns _null_ under a certain condition: 

```cpp
public Column? GetColumnOrNull(string name)
{
  if (string.IsNullOrEmpty(name))
    throw new ArgumentNullException(nameof(name));
  if (_nameMap.TryGetValue(name, out int col))
    return _columns[col];
  return null;
}
```

Immediately after the value is assigned, the _mdType_ variable is accessed to call the _Equals_ method in the condition statement\. The code of this kind may result in an exception\. The developers probably forgot to handle this case with the check\.

### Incorrect order of arguments passed to method

**Issue 4**

```cpp
private IProgressChannel StartProgressChannel(int level)
{
  var newId = Interlocked.Increment(ref _maxSubId);
  return new SubChannel(this, level, newId);
}
```

PVS\-Studio warning: [V3066](https://pvs-studio.com/en/docs/warnings/v3066/)\. Possible incorrect order of arguments passed to method\. ProgressReporter\.cs 148

The analyzer noticed that the order of arguments passed to the method is possibly incorrect\. Indeed, the _StartProgressChannel_ method returns an instance of the _SubChannel_ class, whose constructor received the parameters in the incorrect order \(_id_ and _level_ are mixed up\)\. It's clear from the _SubChannel_ class implementation:

```cpp
public SubChannel(ProgressChannel root, int id, int level)
{
  ....
  id = id;
  level = level;
  ....
}
```

It seems like a simple typo\! Even so, it may cause serious issues\. Pay attention to the _SubChannel_ constructor class implementation\. Here, the _\_id_ and _\_level_ fields are assigned incorrect data\. These fields may have a negative effect on how the class works\.

In fact, [such errors](https://pvs-studio.com/en/blog/examples/v3066/) often occur in large\-scale projects\. As you see, ML\.NET is no exception\. Here's another example:

**Issue 5**

```cpp
internal sealed class LdaSingleBox : IDisposable
{
  ....
  public void AllocateModelMemory(int numTopic, int numVocab, ....)
  {
    ....
    LdaInterface.AllocateModelMemory(...., numVocab, numTopic, ....); // <=
  }
  ....
}
```

PVS\-Studio warning: [V3066](https://pvs-studio.com/en/docs/warnings/v3066/)\. Possible incorrect order of arguments passed to 'AllocateModelMemory' method: 'numVocab' and 'numTopic'\. LdaSingleBox\.cs 142

Here, when the _LdaInterface\.AllocateModelMemory_ method is called, the input parameters \(_numTopic_ and _numVocab_\) are mixed up\. It's clear from the way the parameters were passed to the method's signature:

```cpp
internal static class LdaInterface
{
  ....
  internal static extern void AllocateModelMemory(....,
                                                  int numTopic, // <=
                                                  int numVocab, // <=
                                                  ....);
  ....
}
```

### An object was used after checking for null

**Issue 6**

```cpp
IPredictor IModelCombiner.CombineModels(....)
{
  ....
  foreach (var model in models)
  {
    ....
    var calibrated = predictor as IWeaklyTypedCalibratedModelParameters;
    ....
    if (calibrated != null)
      _host.Check(....);

    predictor = calibrated.WeaklyTypedSubModel; // <=
    paramA = -((PlattCalibrator)calibrated.WeaklyTypedCalibrator).Slope; // <=
    ....
    foreach (var t in tree.TrainedEnsemble.Trees)
    {
      ....
      if (modelCount == 1)
      {
        binaryClassifier = calibrated != null;
        ....
      }
      else
      {
        _host.Check((calibrated != null) == binaryClassifier, ....);
        ....
      }
    }
    ....
  }
}
```

PVS\-Studio warning: [V3125](https://pvs-studio.com/en/docs/warnings/v3125/)\. The 'calibrated' object was used after it was verified against null\. TreeEnsembleCombiner\.cs 54, 58, 59

Here's another way to see a _NullReferenceException_ exception\. Pay attention to the _calibrated_ variable\. This variable is initialized through the _as_ operator, which returns _null_ if the type conversion fails\. This variable is then used unchecked:

```cpp
....
predictor = calibrated.WeaklyTypedSubModel; // <=
paramA = -((PlattCalibrator)calibrated.WeaklyTypedCalibrator).Slope;
....
```

The _calibrated_ variable is checked for _null_ three times in different parts of the method\. We can safely say that this problem is highly relevant for the ML\.NET project\. 

I would note that such errors are most encountered in this project, that's why I suggest considering a few more curious cases:

**Issue 7**

```cpp
private void LoadSampledLabels(....)
{
  ....
  Contracts.Check(_instanceWeights == null || ....);
  if (....)
  {
    for (....)
    {
      ....
      weights[k] = (float)_instanceWeights[j];
    }
  }
  ....
}
```

PVS\-Studio warning: [V3125](https://pvs-studio.com/en/docs/warnings/v3125/)\. The '\_instanceWeights' object was used after it was verified against null\. InternalQuantileRegressionTree\.cs 81, 74

Here, the _null_ check of the _\_instanceWeights_ field is implemented through the _Contracts\.Check_ method: 

```cpp
public static void Check(bool f, string msg)
{
  if (!f)
    throw Except(msg);
}
```

If _\_instanceWeights_ is _null_, the _Contracts\.Check_ method will handle the error by throwing _NullReferenceException_\. It seems to be the developer's initial idea, but things turned out differently\. Note how the _Contracts\.Check_ method is implemented\. If _\_instanceWeights \=\= null_ is true, the _true_ value is passed to the _Contracts\.Check_ method as the _f_ parameter and then the value is converted to _false_\. As a result, the check for _null_ would be incorrect and the code would keep executing\. The next time the _\_instanceWeights_ is accessed, a _NullReferenceException_ would be thrown\. 

**Issue 8**

```cpp
private static List<....> Train(....)
{
  ....
  for (int i = 0; i < columns.Length; i++)
  {
    ....
    var srcColType = inputSchema[srcCol].Type as VectorDataViewType;
    if (srcColType == null || ....) // <=
      throw env.ExceptSchemaMismatch(...., srcColType.ToString()); // <=
    ....
  }
  ....
}
```

PVS\-Studio warning: [V3125](https://pvs-studio.com/en/docs/warnings/v3125/)\. The 'srcColType' object was used after it was verified against null\. Check lines: 843, 842\. LdaTransform\.cs 842, 843

This case struck me as curious since everything seemed to be fine here:

* the type conversion is performed through the _as_ operator, and then the result is written in the _srcColType_ variable;
* the srcColType variable is checked for _null,_ and if the check succeeds, an exception is thrown with the error message\.

And everything would be fine\. However, the result of the _srcColType\.ToString\(\)_ method, called for a potentially empty variable, is passed as the last parameter to the exception: 

```cpp
....
throw env.ExceptSchemaMismatch(...., srcColType.ToString());
....
```

Although this error does not seem extremely serious, it still has a pretty unpleasant consequence: some essential information for debugging \(the error message\) gets lost\. In this case the information is particularly important, since the error is in a loop, and the number of iterations in the loop may reach several dozen\. In similar cases, we need to figure out in what context the error occurred\. 

### An expression is always false

**Issue 9**

```cpp
public static IntArray New(...., IntArrayType type, IntArrayBits bitsPerItem)
{
  ....
  Contracts.CheckParam(type == IntArrayType.Current ||
                       type == IntArrayType.Repeat ||
                       type == IntArrayType.Segmented, nameof(type));

  if (type == IntArrayType.Dense || bitsPerItem == IntArrayBits.Bits0)
  {
    ....
  }
  else if (type == IntArrayType.Sparse)
    return new DeltaSparseIntArray(length, bitsPerItem);
  ....
  return null;
}
```

PVS\-Studio warnings:

* [V3022](https://pvs-studio.com/en/docs/warnings/v3022/)\. Expression 'type \=\= IntArrayType\.Sparse' is always false\. IntArray\.cs 145
* [V3063](https://pvs-studio.com/en/docs/warnings/v3063/)\. A part of conditional expression is always false if it is evaluated: type \=\= IntArrayType\.Dense\. IntArray\.cs 129

In this case, two warnings were triggered by the same cause\. That's another curious case, so let's get into it\. Under given conditions,_ type_ may have _IntArrayType\.Sparse_ or _IntArrayType\.Dense_ value\. But the analyzer finds it wrong\. Is the analyzer right? Let's solve this case\. Look at the following code:

```cpp
Contracts.CheckParam(type == IntArrayType.Current || 
                     type == IntArrayType.Repeat || 
                     type == IntArrayType.Segmented, nameof(type));
```

Here, the _Contracts\.CheckParam_ method is used to check if the _type_ input parameter has one of the following values: _IntArrayType\.Current_, _IntArrayType\.Repeat_, or _IntArrayType\.Segmented_\. 

Now, let's pay attention to the _Contracts\.CheckParam_ method implementation:

```cpp
public static void CheckParam(bool f, string paramName)
{
  if (!f)
    throw ExceptParam(paramName);
}
```

As you can see, if _type_ is not equal to any of the above values, an exception is thrown\.

As a result, we have two sets of values for the _type_ input parameter: 

* expected values \(_IntArrayType\.Sparse_,_ IntArrayType\.Dense_\);
* valid values \(_IntArrayType\.Current_, _IntArrayType\.Repeat_, _IntArrayType\.Segmented_\)\. 

Evidently, the values are completely different, so a serious mistake may have been made here\. The case is solved\!

We could say that the developers did not plan to enumerate all the valid values, when implementing the above check\. But they intended to filter the invalid values for this method\. Therefore, the correct implementation of this check may be as follows:

```cpp
Contracts.CheckParam(type != IntArrayType.Current || 
                     type != IntArrayType.Repeat || 
                     type != IntArrayType.Segmented, nameof(type));
```

## Conclusion

The analyzer detected only a few serious bugs in the ML\.NET project \('human' errors in most cases\)\. So, yes, the Microsoft team does know their job\. But, anyway, even the professionals can make mistakes\. So why not use a static analyzer in your project, thereby minimizing the number of errors and making your projects even better and more reliable?

You can use PVS\-Studio for free by downloading the trial version [here](https://pvs-studio.com/en/pvs-studio/try-free/)\. 

We recommend you to learn the basics of using PVS\-Studio in the [documentation](https://pvs-studio.com/en/docs/) beforehand\. 

After all, I would like to wish the company success in the ML\.NET machine learning framework developing\. I'm highly interested in this project\!

So, I hope you found this article curious :\)\.

Good luck and clean code to you\! Thank you and see you soon\!