Параллельное умножение матриц by Palezehvat · Pull Request #20 · Palezehvat/HomeworksCSharp

Palezehvat · 2023-09-15T16:44:47Z

No description provided.

YuriUfimtsev

Отсутствие метода последовательного умножения матриц и сравнения с параллельным вариантом. Надо дополнить.

YuriUfimtsev · 2023-09-21T07:01:09Z

parallelMatrixMultiplication/parallelMatrixMultiplication/CreateTable.cs

+    /// The function calculates the mathematical expectation and the standard deviation
+    /// </summary>
+    /// <param name="filePath">The file where the results are recorded</param>
+    public static void CreateTableWithResults(string filePath)


Файл с результатами стоит тоже подгрузить

YuriUfimtsev · 2023-09-21T07:12:46Z

parallelMatrixMultiplication/parallelMatrixMultiplication/InvalidFileException.cs

+/// <summary>
+/// Exception for errors with file
+/// </summary>
+public class InvalidFileException : Exception{}


Конструкторов маловато, хотя бы один должен быть

А зачем конструкторы, если они наследуются от Exception, разве нет?

Не совсем, исключение базовых классов не "наследуются" (иначе бы мы могли написать new InvalidFileException("Error message")), но "вызываются" для дочернего класса. Неявно вызываются без параметров.

И сейчас у дочернего класса InvalidFileException тоже есть конструктор по умолчанию без параметров, который выполняется последним среди всей цепочки конструкторов.
Поэтому, конечно, с точки зрения компилируемости и работоспособности кода всё хорошо. Но идеологически, если мы создаём свой exception, то подразумеваем, что оно всеми свойствами исключений обладает: и сообщения передаёт, и внутренние исключения. Поэтому определение конструкторов и входит в exceptions best practices: https://learn.microsoft.com/en-us/dotnet/standard/exceptions/best-practices-for-exceptions.
Хотя на практике конструктор с innerException часто опускается

YuriUfimtsev · 2023-09-21T07:19:06Z

parallelMatrixMultiplication/parallelMatrixMultiplication/Matrix.cs

+    /// <summary>
+    /// A function that receives files with matrices at the input, and a calculated matrix at the output
+    /// </summary>
+    public static void MatrixMultiplicationControlFunction(string firstFile, string secondFile, string resultFile, int sizeThreads)


Тот же MatrixMultiplication для "файловых" матриц. Стоит оформить как перегрузку метода MatrixMultiplication

YuriUfimtsev · 2023-09-21T07:25:20Z

parallelMatrixMultiplication/parallelMatrixMultiplication/Matrix.cs

+    /// <returns>Returns the created matrix</returns>
+    /// <exception cref="ArgumentException">Throws an exception if the number of items 
+    /// in the list does not match the size of the matrix</exception>
+    public static int[][] CreateMatrix(int sizeRows, int sizeColumns, List<int> numbersForMatrix)


Нужно ли здесь слово Matrix? Или пользователь по названию класса сориентируется

Ввод данных через список? А если я ввел значения матрицы 100 * 4 и хочу себя перепроверить. Придется глазами отделять по 4 числа, а где-нибудь к середине вообще случится непреднамеренное смещение на пару чисел. И это еще хорошо, если подряд не будет десятка одинаковых чисел.
Так что нагляднее и дружелюбнее будет списком списков / массивов передавать значения

Скорее эта функция создавалась для тестов и не предполагалась для пользователя, ну если надо и для пользователя, то ок, исправлю

Да, надо исправить, потому что даже для тестов лучше делать визуально понятнее для упрощения как самопроверки, так и проверки другими людьми

YuriUfimtsev · 2023-09-21T07:30:10Z

parallelMatrixMultiplication/parallelMatrixMultiplication/Matrix.cs

+    /// <summary>
+    /// The function compares matrices
+    /// </summary>
+    public static bool MatrixComparison(int[][] firstMatrix, int[][] secondMatrix)


Тот же вопрос по Matrix. И методы лучше глаголами именовать. Здесь, например, Compare

Вообще метод Compare имеет немного другой смысл. Он возвращает число: 0 - объекты равны, x > 0 - первый объект (в каком-то смысле) больше, x < 0 - второй объект больше.
В вашем случае вы просто сравниваете объекты на равенство, поэтому метод лучше назвать AreMatricesEquals

YuriUfimtsev · 2023-09-21T08:03:43Z

parallelMatrixMultiplication/parallelMatrixMultiplication/Matrix.cs

+    /// </summary>
+    /// <returns>Returns the calculated matrix</returns>
+    public static int[][] MatrixMultiplication(int[][] firstMatrix, int[][] secondMatrix, int threadCounts)
+    {


Перекладывать на пользователя ответственность за количество потоков, конечно, можно. Но обрадуется ли он, если введет 5, а на самом деле свободно 6 (Environment.ProccesorCount вернет 6), и один поток будет простаивать (а пользователь скучать лишние n минут). Или если введет 50, ожидая, что заварит чай и увидит результат умножения огромных матриц, а просидит еще дольше, потому что доступных потоков будет сильно меньше, и его 50 штук будут толпиться, забирая время на переключение между потоками.
Так что если стремимся к эффективности и меньшему ожиданию результата, то число потоков надо брать из Environment.ProcessorCount

YuriUfimtsev · 2023-09-21T08:25:33Z

parallelMatrixMultiplication/parallelMatrixMultiplication/Matrix.cs

+        var allLines = File.ReadAllLines(filePath);
+
+        var matrix = new int[allLines.Length][];
+
+        var sizeColumns = 0;


Suggested change

var allLines = File.ReadAllLines(filePath);

var matrix = new int[allLines.Length][];

var sizeColumns = 0;

var allLines = File.ReadAllLines(filePath);

var matrix = new int[allLines.Length][];

var sizeColumns = 0;

YuriUfimtsev · 2023-09-21T08:43:17Z

parallelMatrixMultiplication/parallelMatrixMultiplication/Matrix.cs

+            start = end;
+        }
+
+        foreach (var thread in threads )


Suggested change

foreach (var thread in threads )

foreach (var thread in threads)

YuriUfimtsev · 2023-09-21T08:50:23Z

parallelMatrixMultiplication/parallelMatrixMultiplication/CreateTable.cs

+/// <summary>
+/// A class for measuring the standard deviation and mathematical expectation 
+/// </summary>
+public static class CreateTable


Так все-таки класс для измерения отклонения и ожидания или для создания таблиц? :)

YuriUfimtsev · 2023-09-21T08:58:14Z

parallelMatrixMultiplication/parallelMatrixMultiplication/Program.cs

+Console.WriteLine("Write file path for create table");
+var tableFile = Console.ReadLine();
+
+if (tableFile != null && firstFile != null && secondFile != null && resultFile != null)


Стоит сообщать о результате: записалось в файл / не записалось (и в чем тогда проблема)

YuriUfimtsev · 2023-09-29T10:37:46Z

parallelMatrixMultiplication/parallelMatrixMultiplication/CreateTable.cs

 /// A class for measuring the standard deviation and mathematical expectation 
 /// </summary>
-public static class CreateTable
+public static class GetStandartDeviationAndMinValue


Файл тоже надо переименовать. И лучше классы называть существительными, "StandartDeviationAndMathExpectationMeasurements"

Кстати, MinValue хорошо бы поменять в коде на MathExpectation (или объяснить, если это не одно и то же)

YuriUfimtsev · 2023-09-29T10:38:51Z

parallelMatrixMultiplication/tableFile.txt

@@ -0,0 +1,13 @@
+В файле представлены: математическое ожидание и среднеквадратичное отклонение


Так лучше, но все равно стоит привести к виду, который просили по условию.

Показательнее привести результаты для достаточно больших размеров матриц (например, брать минимальный размер 500x500)

Файл преобразовать в табличный вид

Что имеется в виду под табличным видом? И есть же отдельный пример для больших матриц: 10000x10000 и 10000x1

Табличный вид: сверху названия величин, ниже результаты измерений. Самый простой способ: текстовый файл с разделителями для ячеек таблицы, либо с аккуратным выравниванием.

Один пример для больших матриц есть, надо больше, см. условие.

YuriUfimtsev · 2023-09-29T10:45:40Z

parallelMatrixMultiplication/parallelMatrixMultiplication/Program.cs

+}
+if (resultFile == null)
+{
+    Console.WriteLine("Неверно прописан путь для firstFile");


для resultFile

YuriUfimtsev · 2023-09-29T10:45:56Z

parallelMatrixMultiplication/parallelMatrixMultiplication/Program.cs

+}
+if (firstFile == null)
+{
+    Console.WriteLine("Неверно прописан путь для resultFile");


YuriUfimtsev · 2023-09-29T11:01:27Z

parallelMatrixMultiplication/parallelMatrixMultiplication/CreateTable.cs

+        listOfValuesSecondMatrix.Add(new int[1] { 1 });
+        listOfValuesSecondMatrix.Add(new int[1] { 2 });
+        listOfValuesSecondMatrix.Add(new int[1] { 0 });
+        listOfValuesSecondMatrix.Add(new int[1] { 0 });
+        listOfValuesSecondMatrix.Add(new int[1] { 5 });


Можно проще (здесь и в других местах)

Suggested change

listOfValuesSecondMatrix.Add(new int[1] { 1 });

listOfValuesSecondMatrix.Add(new int[1] { 2 });

listOfValuesSecondMatrix.Add(new int[1] { 0 });

listOfValuesSecondMatrix.Add(new int[1] { 0 });

listOfValuesSecondMatrix.Add(new int[1] { 5 });

var listOfValuesSecondMatrix = new List<int[]>

{

new int[1] { 1 },

new int[1] { 2 },

new int[1] { 0 },

new int[1] { 0 },

new int[1] { 5 },

}

YuriUfimtsev · 2023-09-29T11:14:19Z

parallelMatrixMultiplication/parallelMatrixMultiplication/CreateTable.cs

+    }
+
+    private static void MultiplyingMatricesWithNegativeNumbers(string filePath)
+    {


Private-методы лучше тоже глаголами именовать. MultiplyMatricesWithNegativeNumbers (и соседние методы аналогично)

YuriUfimtsev · 2023-09-29T11:15:27Z

parallelMatrixMultiplication/parallelMatrixMultiplication/Matrix.cs

+        return resultMatrix;
+    }
+
+    private static int[][] ConvertinMatrixToTransposedOne(int[][] matrix)


ConvertMatrixToTransposedOne

YuriUfimtsev · 2023-09-29T11:20:05Z

parallelMatrixMultiplication/parallelMatrixMultiplication/Matrix.cs

+    /// <returns>Returns the created matrix</returns>
+    /// <exception cref="ArgumentException">Throws an exception if the number of items 
+    /// in the list does not match the size of the matrix</exception>
+    public static int[][] Create(int sizeRows, int sizeColumns, List<int[]> numbersForMatrix)


Вот, Matrix.Create гораздо приятнее читается :)
Ниже тоже можно без Matrix в названиях методов обходиться

YuriUfimtsev · 2023-10-01T20:31:29Z

...llelMatrixMultiplication/parallelMatrixMultiplication/StandartDeviationAndMathExpectation.cs

+            stopWatch.Start();
+            var resultMatrix = Matrix.Multiply(firstMatrix, secondMatrix);
+            stopWatch.Stop();
+            arrayForStandardDeviation[i] = (double)stopWatch.ElapsedMilliseconds / 1000;


Зачем здесь на тысячу деление?

YuriUfimtsev · 2023-10-01T20:31:57Z

parallelMatrixMultiplication/tableFile.txt

@@ -0,0 +1,13 @@
+В файле представлены: математическое ожидание и среднеквадратичное отклонение
+Математическое ожидание и среднеквадратичное отклонение для матриц размера 3x3:
+Матемматическое ожидание: 0,0019000000000000002 Среднеквадратичное отклонение: 0,002514402955419481


Suggested change

Матемматическое ожидание: 0,0019000000000000002 Среднеквадратичное отклонение: 0,002514402955419481

Математическое ожидание: 0,0019000000000000002 Среднеквадратичное отклонение: 0,002514402955419481

YuriUfimtsev · 2023-10-01T20:35:42Z

parallelMatrixMultiplication/tableFile.txt

@@ -0,0 +1,13 @@
+В файле представлены: математическое ожидание и среднеквадратичное отклонение


Табличный вид: сверху названия величин, ниже результаты измерений. Самый простой способ: текстовый файл с разделителями для ячеек таблицы, либо с аккуратным выравниванием.

Один пример для больших матриц есть, надо больше, см. условие.

YuriUfimtsev · 2023-10-09T18:29:12Z

parallelMatrixMultiplication/tableFile.txt

+ нулями			0,1		0,3162278
+
+500x500
+и 500x500		106,3		4,6916001


Да, хотя бы с двумя примерами для больших матриц.
Также надо разделить результаты (матожидание и отклонение) на параллельный и последовательный варианты: то, ради чего эти все измерения затевались.
А измерения для маленьких матриц лучше вообще убрать: непоказательно, так еще и среднеквадратичное отклонение больше матожидания

YuriUfimtsev

Хорошо, но пару вещей поправить осталось

YuriUfimtsev · 2023-10-13T21:37:36Z

parallelMatrixMultiplication/tableFile.txt

+
+500x500
+и 500x500		129,5										29,9007618						614,8							7,4654761
+


Столбец с матожиданием под размеры матриц уехал, и названия друг под другом сидят. Можно их как-нибудь сократить, например, английский язык использовать или греческие буквы, общепринятые для этих величин.
А с длинными фразами про параллельность и последовательность можно так поступить: текст на строке "параллельный метод умножения", ниже все данные, включая размеры, ниже строчка с объявлением последовательного способа и снова все данные в ячейках.
Важно, чтобы значения друг к другу не липли, и названия столбцов хотя бы на расстоянии одного пробела друг от друга находились

Я не знаю, где вы открываете файл или смотрите его, однако у меня нету переноса 3 и др длинных строк в обычном блокноте просто обычные строки, хоть и большой длины, так стоят ли так сильно заморачиваться с таблицей если только github или там где вы открывали, переносит строчки? Кроме того при запуске программы сгенерится txt'шник, который можно открыть в блокноте и там всё будет ок.

В любом случае, при незначительном уменьшении окна значения таблицы файла не должны так сильно съезжать. Что если у пользователя другое разрешение экрана? Он будет вынужден анализировать ваш съехавший файл

Ок, я понял, исправлю. Однако в коде нет переносов строки, которые ломают таблицу, значит переносит либо текстовый редактор, либо у пользователя выставлено сильное приближение экрана, так разве неважно насколько небольшие столбцы, найдётся пользователь у которого всё равно будет съехавшая таблица, почему мы не можем переложить вину на пользователя в этом случае?

Хорошо, давайте просто добьемся того, чтобы при открытии файла на весь экран все выглядело хорошо: под каждым названием было свое значение, названия друг на друга не наезжали, и далее в каждой (одной) строке находилось по 5 значений.
Ключевая проблема того, что у вас сейчас даже при просмотре в полноэкранном режиме название одного столбца съезжает, и значения скачут по разным строчкам в том, что у вас сильно много табуляций стоит между значениями, и названия столбцов длинные. Поменяйте это и всё должно стать хорошо :)

YuriUfimtsev · 2023-10-13T21:42:49Z

parallelMatrixMultiplication/TestsForMatrixMultiplication/TestsForMatrixMultiplication.cs

+        var correctMatrix = Matrix.Create(3, 3, listOfCorrectValues);
+
+        var resultMatrix = Matrix.Multiply(firstMatrix, secondMatrix);
+        Assert.True(Matrix.AreEquals(resultMatrix, correctMatrix));


Еще у вас в тестах не заметил сравнения результатов последовательного умножения CoherentMultiply с параллельным (который почему-то просто Multiply, лучше тогда тоже ParallelMultiply назвать).
Так что добавьте, пожалуйста, проверки в тестах на то, что другой метод умножения тот же результат дает

Параллельное умножение матриц

e71ae58

YuriUfimtsev reviewed Sep 21, 2023

View reviewed changes

Изменения по ревью

b2d9c13

YuriUfimtsev reviewed Sep 29, 2023

View reviewed changes

Ревью

ef7f6c0

YuriUfimtsev reviewed Oct 1, 2023

View reviewed changes

Ревью

0c93f36

YuriUfimtsev reviewed Oct 9, 2023

View reviewed changes

Ревью

9685a1d

YuriUfimtsev reviewed Oct 13, 2023

View reviewed changes

palorel added 2 commits October 14, 2023 15:14

Ревью

bd2282f

Ревью

8d4757b

	foreach (var thread in threads )
	foreach (var thread in threads)

		@@ -0,0 +1,13 @@
		В файле представлены: математическое ожидание и среднеквадратичное отклонение

-        listOfValuesSecondMatrix.Add(new int[1] { 1 });
-        listOfValuesSecondMatrix.Add(new int[1] { 2 });
-        listOfValuesSecondMatrix.Add(new int[1] { 0 });
-        listOfValuesSecondMatrix.Add(new int[1] { 0 });
-        listOfValuesSecondMatrix.Add(new int[1] { 5 });
+var listOfValuesSecondMatrix = new List<int[]>
+{
+    new int[1] { 1 },
+    new int[1] { 2 },
+    new int[1] { 0 },
+    new int[1] { 0 },
+    new int[1] { 5 },
+}

	Матемматическое ожидание: 0,0019000000000000002 Среднеквадратичное отклонение: 0,002514402955419481
	Математическое ожидание: 0,0019000000000000002 Среднеквадратичное отклонение: 0,002514402955419481

Conversation

Palezehvat commented Sep 15, 2023

Uh oh!

YuriUfimtsev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriUfimtsev Sep 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriUfimtsev Sep 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriUfimtsev Sep 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriUfimtsev Oct 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriUfimtsev Oct 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriUfimtsev Oct 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriUfimtsev Sep 23, 2023 •

edited

Loading

YuriUfimtsev Sep 29, 2023 •

edited

Loading

YuriUfimtsev Sep 29, 2023 •

edited

Loading

YuriUfimtsev Oct 1, 2023 •

edited

Loading

YuriUfimtsev Oct 1, 2023 •

edited

Loading

YuriUfimtsev Oct 9, 2023 •

edited

Loading

YuriUfimtsev Oct 13, 2023 •

edited

Loading

YuriUfimtsev Oct 13, 2023 •

edited

Loading