ermig1979 / Simd

C++ image processing and machine learning library with using of SIMD: SSE, AVX, AVX-512, AMX for x86/x64, NEON for ARM.
http://ermig1979.github.io/Simd
MIT License
2.06k stars 412 forks source link

Ошибка в коде к статье на Habr #129

Closed GlebSBrykin closed 3 years ago

GlebSBrykin commented 4 years ago

Здравствуйте! Хочу сообщить об ошибке в коде к Вашей статье https://m.habr.com/ru/post/448436/, а именно в базовой реализации свёртки. depthwise свёртка(с group > 1) вычисляется некорректно: значения есть только у первых каналов, последующие же каналы заполнены нулями.

ermig1979 commented 4 years ago

Здравствуйте! В коде который в статье - я специально сильно сокращал, что бы он влез в формат статьи. Видимо перестарался. Ошибочный вариант в формате NCHW или NHWC?

GlebSBrykin commented 4 years ago

В NCHW. Для обычной свёртки работает корректно(сверял с PyTorch), а вот depthwice заполняет только первый канал(первый заполняет правильно), последующие заполнены нулями.

GlebSBrykin commented 4 years ago

Обнаружил ещё одну ошибку в оптимизированном коде к той же статье. Формат NCHW. Обычная свёртка работает правильно, depthwise вылетает при попытке доступа в защищённую память(выход за границы массива если использовать указатели).

ermig1979 commented 4 years ago

Спасибо за сообщение об ошибке. Исправил.

GlebSBrykin commented 4 years ago

К сожалению, ошибка сохраняется.

GlebSBrykin commented 4 years ago

Я запускаю этот код в C#, т.к. это позволяет проще находить ошибки. Из изменений - только удаление модивикатора const в параметрах функций, всё остальное - как на Хабре. Код скопировал сразу после Вашего сообщения. Прикрепляю файл с полным кодом для воспроизведения ошибки. Хочу отметить, что обычная свёртка с разными разерами шага работает корректно.

ermig1979 commented 4 years ago

По ваше ссылке говорит, что ничего не найдено.

ermig1979 commented 4 years ago

Впрочем еще одну ошибку я нашел.

GlebSBrykin commented 4 years ago

Ошибки в быстрой реализации исправлены. Прошу прощения, не заметил у себя ошибку в тестовом коде.

GlebSBrykin commented 4 years ago

И всё же ошибка есть... На маленьких размерах данных быстрый алгоритм(через матричное умножение) работает корректно для всех проверенных условий. Результаты работы алгоритма совпадают с результатами в PyTorch. Однако стоит увеличить объемы данных до адекватных размеров, как появляются несоответствия с PyTorch. Я интегрировал Ваш код в своё работающее приложение для стилизации изображений и получил некорректный результат. Если необходимо, могу предоставить все исходные коды.

GlebSBrykin commented 4 years ago

Под маленькими размерами данных понимается размер входного массива 1 3 5 5 и 3 3 3 3 для весов свёртки. Под реальными размерами - 1 128 256 256 для входного массива и 128 128 3 3 для массива весов.

ermig1979 commented 4 years ago

А на сколько некорректный?

GlebSBrykin commented 4 years ago

Совсем некорректный. Вот, для примера, результат: # IMG_20200922_095155 Это я получаю вместо нормального изображения. Вот здесь: https://github.com/ColorfulSoft/StyleTransfer-Colorization-SuperResolution/tree/master/Style%20Transfer/2017.%20Arbitrary%20Style%20Transfer%20in%20Real-time%20with%20Adaptive%20Instance%20Normalization можно посмотреть на то, что получается с обычной свёрткой. Странно, что на небольших объёмах данных Ваш код работает правильно, а на больших - нет. Я пока даже не могу предположить, в чём может быть проблема. Ошибок выхода за границы массива не возникало. Причём одинаковая картина и с Вашей версией базовой реализации свёрки и с быстрой.

GlebSBrykin commented 4 years ago

Может быть, проблема в .NET, но чтобы это проверить, необходимо запустить Ваш код на C++.

ermig1979 commented 4 years ago

Хорошо. Как будет свободное время напишу тесты для С++ версии.

GlebSBrykin commented 4 years ago

Ошибка в .NET крайне маловероятна. Сейчас проверил свою реализацию свёртки с указателями и никаких проблем не наблюдаю.

ermig1979 commented 4 years ago

А у вас какой размер внешнего буфера? Требуется как минимум srcC*kernelY*kernelX*dstH*dstW. Просто, если на малых размерах работает, а на больших нет, то только две возможные причины: переполнение переменных (32 bit int вроде не переполняется) и переполнение буфера.

GlebSBrykin commented 4 years ago

Такой и есть. Вчера ещё несколько тестов провёл, полное совпадение с PyTorch во всех проверенных случаях(разница отдельных значений менее 0.001), но стилизация пока что не работает. Буду дальше искать решение.

GlebSBrykin commented 4 years ago

Переполнения int32 не было, более того, быть не могло, т.к. в C# создать массив длиной больше максимального значения int простым способом невозможно. Попытка сделать это вызывает исключение. А выделение памяти я провожу как раз через создание массивов и последующее получение указателей на них. Да и проверку на переполнение я включал. Переполнений не было.

GlebSBrykin commented 4 years ago

Ещё хотел бы спросить про обратное распространение ошибки через быструю свёртку. Я прикинул и получил такие формулы(в терминологии Вашей статьи):

dA = C B^t dB = A^t C При этом dB потребует преобразование, обратное im2col.

Я прав?

GlebSBrykin commented 4 years ago

А у вас какой размер внешнего буфера? Требуется как минимум srcC*kernelY*kernelX*dstH*dstW. Просто, если на малых размерах работает, а на больших нет, то только две возможные причины: переполнение переменных (32 bit int вроде не переполняется) и переполнение буфера.

В таком случае в статье есть ещё одна ошибка: 2lcmlpitdsgjyztxnyylyqi62jo Ведь исходя из изображения, количество элементов буффера делится на group, однако этого количества недостаточно: в программе возникает ошибка.

Ещё одна ошибка непосредственно в коде: В void convolution:

im2row(src, srcC, srcH, srcW, kernelY, kernelX, dilationY, dilationY,
      strideY, strideX, padY, padX, padH, padW, group, buf);

Два dilationY вместо dilationY, dilationX.

ermig1979 commented 4 years ago

В буфере не надо делить количество элементов на группы. Функции im2col и im2row преобразуют входное изображение целиком. P.S. Исправил ошибку с dilationX.