# ML\.NET: можно ли доверять машинному обучению Microsoft?

В 2018 году Microsoft разработали ML\.NET – фреймворк машинного обучения для \.NET разработчиков\. За прошедшее время эта библиотека претерпела существенные изменения и обзавелась новыми функциями для выявления закономерностей в данных\. Посмотрим, как это отразилось на качестве её исходного кода\.

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

## Введение

Перед началом разбора ошибок предлагаю познакомиться с самим фреймворком\. ML\.NET — разработанная компанией Microsoft бесплатная Open Source библиотека со средствами машинного обучения для языков программирования C\# и F\#\. Она позволяет создавать функционал для получения автоматических прогнозов на основе закономерностей в данных, доступных вашему приложению\.

С помощью ML\.NET можно обучить модель машинного обучения, указав соответствующий алгоритм, или импортировать предварительно обученные модели TensorFlow и ONNX\.

Обученную модель можно добавить в приложение и использовать её для получения прогнозов\.

На момент написания этой статьи ML\.NET включает в себя средства для решения следующих задач: классификация/категоризация, регрессия данных, обнаружение аномалий в данных, реализация рекомендательных систем, классификация изображений, анализ последовательностей и временных рядов\.

Познакомившись с проектом, можно перейти к основной теме – разбору найденных ошибок\. Но сначала хочу отдать должное разработчикам Microsoft — проект выполнен весьма качественно, найти что\-то страшное и интересное \(в рамках тематики этой статьи\) было непросто\. Тем не менее есть несколько серьёзных моментов, заслуживающих внимания\. 

Проверка проводилась на релизе ML\.NET 1\.7\.1\. Исходный код данной версии проекта доступен на [GitHub](https://github.com/dotnet/machinelearning/releases/tag/v1.7.1)\.

Итак, приступаем к разбору\!

## Разбор ошибок в ML\.NET

### Объект использовался после проверки на null

**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: [V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/)\. The 'estimators' object was used after it was verified against null\. EstimatorChain\.cs 37, 39

Рассмотрим первое предупреждение\. В данном случае PVS\-Studio обнаружил потенциальную ошибку, которая может привести к разыменованию нулевой ссылки\. Обратите внимание на код\. С помощью оператора null\-объединения выполняется проверка входного параметра _estimators_ на наличие значения, после чего результат работы оператора записывается в поле _\_estimators_\. 

Строчкой ниже разработчик снова обращается к параметру_ estimators_, вызывая метод _LastOrDefault\. _Т\. к\. _estimators _может иметь значение null, при обращении к этому параметру есть риск получить исключение_ NullReferenceException\._

Решением данной проблемы является использование поля _\_estimators_ для вызова метода_ LastOrDefault _вместо _estimators:_

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

Имена параметра метода и поля класса очень похожи\. Можно предположить, что разработчик их просто перепутал\. Подобная мелочь запросто ускользнёт от взора разработчика при написании кода\. Статический анализатор обращает наше внимание на такие моменты, тем самым избавляя от необходимости искать и исправлять эти ошибки в будущем\.

### Два оператора 'if' с одинаковыми условными выражениями

**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: [V3021](https://pvs-studio.ru/ru/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

Анализатор обнаружил два похожих оператора _if_ с идентичными условными выражениями\. В блоке обоих выражений вызывается исключение, тем самым прерывая выполнение программы\. Возможно, это выглядит лишь как ненужное нагромождение кода, однако я предлагаю взглянуть на проблему с другой стороны\. Посмотрите на сообщения об ошибках – они не совпадают\. Логично предположить, что разработчик собирался реализовать две различных проверки\. Он скопировал одну из проверок, заменил сообщение, но забыл исправить само условие\. Таким образом, одна из возможных ошибок осталась необработанной и всё также представляет угрозу для корректной работы программы\.

### Переменная использовалась после того, как она была назначена с помощью null\-условного оператора

**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: [V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/) The 'mdType' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. PipelineEnsemble\.cs 670

В этот раз анализатор PVS\-Studio предупреждает о возможном возникновении _NullReferenceException_ при разыменовании переменной, которая может иметь значение _null_\. Анализатор сделал такой вывод из\-за отсутствия соответствующей проверки между присваиванием переменной значения, потенциально равного _null_, и её непосредственным использованием\. Обратите внимание на код\. Переменной _mdType_ присваивается значение посредством цепочки вызовов\. Одним из её звеньев является метод _GetColumnOrNull_, который при определённом условии возвращает значение _null_:

```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;
}
```

Сразу после присваивания значения в условии выполняется обращение к переменной _mdType_ для вызова метода _Equals_\. Такой код может привести к возникновению исключения\. Скорее всего, разработчик просто забыл обработать этот случай соответствующей проверкой\. 

### Неправильный порядок аргументов, передаваемых методу

**Issue 4**

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

Предупреждение PVS\-Studio: [V3066](https://pvs-studio.ru/ru/docs/warnings/v3066/)\. Possible incorrect order of arguments passed to method\. ProgressReporter\.cs 148

Анализатор сообщает о возможном неправильном порядке аргументов, передаваемых методу\. Действительно, метод _StartProgressChannel_ возвращает экземпляр класса _SubChannel_, в конструктор которого были переданы параметры в неправильном порядке \(_id_ и _level_ перепутаны местами\)\. В этом легко убедиться, если посмотреть на реализацию конструктора класса _SubChannel_:

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

Казалось бы, простая опечатка\! И всё же она может стать источником серьёзных проблем\. Обратите внимание на реализацию конструктора класса _SubChannel_\. Здесь полям _\_id_ и _\_level_ присваиваются неверные данные\. Эти поля, в свою очередь, могут быть использованы в остальном функционале класса, тем самым нарушая его работу\. 

К слову, [подобные ошибки](https://pvs-studio.ru/ru/blog/examples/v3066/) – вовсе не редкость и частенько встречаются в крупных проектах\. Как вы поняли, ML\.NET — не исключение\. Вот ещё один пример:

**Issue 5**

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

Предупреждение PVS\-Studio: [V3066](https://pvs-studio.ru/ru/docs/warnings/v3066/)\. Possible incorrect order of arguments passed to 'AllocateModelMemory' method: 'numVocab' and 'numTopic'\. LdaSingleBox\.cs 142

Здесь при вызове метода _LdaInterface\.AllocateModelMemory _были перепутаны местами входные параметры: _numTopic_ и _numVocab_\. В этом нетрудно убедиться, взглянув на их порядок в сигнатуре метода:

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

### Объект использовался после проверки на 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: [V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/)\. The 'calibrated' object was used after it was verified against null\. TreeEnsembleCombiner\.cs 54, 58, 59

Представляю вам ещё один способ получения исключения типа _NullReferenceException_\. Обратите внимание на переменную _calibrated_\. Она инициализируется с помощью оператора _as_, который при неудачном приведении к типу возвращает _null_\. Далее эта переменная используется без какой\-либо проверки:

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

Также хочу отметить количество проверок _calibrated_ на _null_: в разных частях метода эта переменная проверяется трижды\. Даже без подробного разбора кода можно сделать уверенный вывод: обсуждаемая проблема реальна и заслуживает отдельного внимания со стороны разработчика\. 

Замечу, что ошибки такого типа наиболее распространены в этом проекте, а потому предлагаю рассмотреть ещё парочку интересных примеров:

**Issue 7**

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

Предупреждение PVS\-Studio: [V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/)\. The '\_instanceWeights' object was used after it was verified against null\. InternalQuantileRegressionTree\.cs 81, 74

В данном случае проверка поля _\_instanceWeights_ на _null_ реализована с помощью метода _Contracts\.Check_: 

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

Если _\_instanceWeights_ равен _null_, в методе _Contracts\.Check_ будет выполнена обработка ошибки с помощью соответствующего исключения\. Скорее всего, именно так и задумывал автор, но в реальности получилось иначе\. Обратите внимание на реализацию метода _Contracts\.Check_\. В случае, когда выполняется условие _\_instanceWeights \=\= null_, в метод _Contracts\.Check_ будет передано значение _true_ в качестве параметра _f_ и преобразовано в _false_\. В результате проверка на _null_ отработает некорректно, код продолжит выполняться, а при следующем обращении к полю _\_instanceWeights_ будет вызвано исключение _NullReferenceException_ 

**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: [V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/)\. The 'srcColType' object was used after it was verified against null\. Check lines: 843, 842\. LdaTransform\.cs 842, 843

Данный случай показался мне интересным, ведь, на первый взгляд здесь всё в порядке:

* выполняется приведение типов с помощью оператора _as_, а затем результат записывается в переменную _srcColType_;
* переменная srcColType проверяется на _null,_ и, если проверка проходит успешно, вызывается исключение с соответствующим сообщением об ошибке\.

И всё было бы отлично, только вот в качестве последнего параметра в исключении используется результат метода _srcColType\.ToString\(\)_, вызываемого у потенциально пустой переменной:

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

Хоть эта ошибка и не кажется очень опасной, одно неприятное последствие у неё есть – теряется ценная для отладки информация \(сообщение об ошибке\)\. В данном случае это особенно важно, ведь ошибка находится в цикле, число итераций которого может достигать нескольких десятков\. В подобных случаях необходимо иметь представление, при каких условиях возникла ошибка\. 

### Выражение всегда ложно

**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:

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

В данном случае было получено сразу два предупреждения, причём оба получены по одной и той же причине\. Случай интересный, поэтому перейдём к делу\. В приведённых условиях ожидается, что _type_ может иметь значения _IntArrayType\.Sparse_ и _IntArrayType\.Dense_, но наш анализатор с этим совершенно не согласен\. Прав ли он? Предлагаю это выяснить\. Обратите внимание на следующий код:

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

Здесь метод _Contracts\.CheckParam_ используется для проверки входного параметра _type_ на равенство одному из следующих значений: _IntArrayType\.Current_, _IntArrayType\.Repeat_ или _IntArrayType\.Segmented_\. 

Теперь взглянем на реализацию метода _Contracts\.CheckParam_:

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

Как видно, в случае если _type_ не равен ни одному из приведенных выше значений, вызывается исключение\.

В итоге мы имеем два набора значений входного параметра _type_: 

* ожидаемые значения \(_IntArrayType\.Sparse_,_ IntArrayType\.Dense_\);
* допустимые значения \(_IntArrayType\.Current_, _IntArrayType\.Repeat_, _IntArrayType\.Segmented_\)\. 

Очевидно, что они совершенно не совпадают, а в коде, возможно, допущена серьёзная ошибка\. Дело раскрыто\!

Можно предположить, что при реализации приведённой выше проверки автор кода вовсе не ставил перед собой задачу перечислить все допустимые значения\. Совсем наоборот, он хотел отфильтровать значения, недопустимые в данном методе\. Исходя из этого, правильная реализация этой проверки могла бы выглядеть так:

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

## Заключение

Серьёзных ошибок в проекте было обнаружено немного\. А те, что были найдены, скорее допущены по невнимательности, нежели из\-за недостатков в навыках разработчиков\. Это показывает, что компания Microsoft знает свое дело, но также очередной раз доказывает, что профессионалам тоже свойственно ошибаться\. Так почему бы не использовать статический анализатор в своей работе, тем самым сведя количество ошибок к минимуму и сделав свои проекты ещё качественнее и надёжнее?

Если вы согласны со мной, можете попробовать PVS\-Studio в течение бесплатного периода, скачав пробную версию [здесь](https://pvs-studio.ru/ru/pvs-studio/try-free/)\. 

Для наилучшего пользовательского опыта рекомендую уделить немного времени и ознакомиться с основами использования анализатора, прочитав интересующие вас разделы в [документации](https://pvs-studio.ru/ru/docs/)\.

Напоследок хочу пожелать компании Microsoft успехов в работе над фреймворком машинного обучения ML\.NET, я с интересом наблюдаю за его развитием\!

На этом статья завершается, надеюсь она показалась вам интересной :\)\.

Чистого кода и успешных проектов\! До встречи в следующих статьях\!