Matrix Multiplication #1
Conversation
| using System.IO; | ||
| using NUnit.Framework; | ||
|
|
||
| namespace MatrixMultiplication.Tests; |
There was a problem hiding this comment.
Лучше namespace писать первой строкой файла (сразу после шапки с лицензией, которую мы по традиции не пишем почему-то). Это более консистентно с синтаксисом других языков типа Java, и в целом более аккуратно.
| var matrix2 = new DenseMatrix("matrix2.txt"); | ||
| Assert.AreEqual(matrix1.Matrix, new[,] { { 10, 4, 5 }, { 8, 9, 1 } }); | ||
| Assert.AreEqual(matrix2.Matrix, new[,] { { 2, 9 }, { 1, 7 }, {4, 19} }); | ||
| } |
There was a problem hiding this comment.
Неплохо бы, чтобы тест удалял создаваемые им файлы. Во избежание случайных зависимостей тестов по данным.
| { | ||
| var correctResult = new[,] { { 44, 213 }, { 29, 154 } }; | ||
| var matrix1 = new DenseMatrix(new[,] { { 10, 4, 5 }, { 8, 9, 1 } }, "matrix1.txt"); | ||
| var matrix2 = new DenseMatrix(new[,] { { 2, 9 }, { 1, 7 }, {4, 19} }, "matrix2.txt"); |
There was a problem hiding this comment.
| var matrix2 = new DenseMatrix(new[,] { { 2, 9 }, { 1, 7 }, {4, 19} }, "matrix2.txt"); | |
| var matrix2 = new DenseMatrix(new[,] { { 2, 9 }, { 1, 7 }, {4, 19 } }, "matrix2.txt"); |
| var matrix2 = new DenseMatrix(new[,] { { 100 } }, "matrix2.txt"); | ||
| var result1 = DenseMatrix.MultiplyConcurrently(matrix1, matrix2); | ||
| var result2 = DenseMatrix.MultiplySerially(matrix1, matrix2); | ||
| Assert.AreEqual(result1.Matrix, new[,] { {900} }); |
There was a problem hiding this comment.
| Assert.AreEqual(result1.Matrix, new[,] { {900} }); | |
| Assert.AreEqual(result1.Matrix, new[,] { { 900 } }); |
| /// <param name="N"></param> | ||
| /// <param name="calculationTime"></param> | ||
| /// <returns></returns> |
| for (var r = 0; r < numberOfRows; r++) | ||
| { | ||
| var line = Array.ConvertAll(fileEnumerator.Current.Split(), int.Parse); | ||
| for (var c = 0; c < numberOfColumns; c++) |
There was a problem hiding this comment.
Так лучше их и называть, row и column
| } | ||
| } | ||
|
|
||
| file.Close(); |
There was a problem hiding this comment.
Вообще, using его сам закроет, но так тоже ок.
| { | ||
| using var file = new StreamWriter(File.Create(path)); | ||
|
|
||
| Random rnd = new Random(); |
There was a problem hiding this comment.
| Random rnd = new Random(); | |
| var rnd = new Random(); |
| /// <returns>Result of matrix multiplication.</returns> | ||
| public static DenseMatrix MultiplyConcurrently(DenseMatrix matrix1, DenseMatrix matrix2) | ||
| { | ||
| var numberOfThreads = Environment.ProcessorCount / 3; |
There was a problem hiding this comment.
А если матрицы размером меньше, чем numberOfThreads?
| @@ -0,0 +1,12 @@ | |||
| | size | Concurrent Multiplication | Serial Multiplication | | |||
| |---------|------------------|------------------------|------------------|------------------------| | |||
| | | Expected Value | Standard Deviation | Expected Value | Standard Deviation | | |||
There was a problem hiding this comment.
Значения без единиц измерения бессмысленны
| { | ||
| var correctResult = new[,] { { 44, 213 }, { 29, 154 } }; | ||
| var matrix1 = new DenseMatrix(new[,] { { 10, 4, 5 }, { 8, 9, 1 } }); | ||
| var matrix2 = new DenseMatrix(new[,] { { 2, 9 }, { 1, 7 }, {4, 19} }); |
There was a problem hiding this comment.
| var matrix2 = new DenseMatrix(new[,] { { 2, 9 }, { 1, 7 }, {4, 19} }); | |
| var matrix2 = new DenseMatrix(new[,] { { 2, 9 }, { 1, 7 }, { 4, 19 } }); |
| /// </summary> | ||
| /// <param name="calculationTime">Array, which elements are time of matrix multiplications.</param> | ||
| /// <returns>Returns long value -- expected value of the given data.</returns> | ||
| public static long CalculateExpectedValue(long[] calculationTime) => calculationTime.Sum() / calculationTime.Length; |
There was a problem hiding this comment.
В LINQ есть IEnumerable.Average, он бы сам это сделал
|
|
||
| using System.Linq; | ||
|
|
||
| public class DenseMatrix |
| { | ||
| using var file = new StreamWriter(File.Create(path)); | ||
|
|
||
| var rnd = new Random(); |
There was a problem hiding this comment.
Лучше random сделать статическим полем и инициализировать один раз.
| @@ -0,0 +1,40 @@ | |||
| namespace MatrixMultiplication; | |||
|
|
|||
| public static class Table | |||
| |---------|------------------|------------------------|------------------|------------------------| | ||
| | |Expected Value, ms| Standard Deviation, ms |Expected Value, ms| Standard Deviation, ms | | ||
| |---------|------------------|------------------------|------------------|------------------------| | ||
| | 100x100 |3 |1,0488088481701514 |12 |0,4472135954999579 | |
There was a problem hiding this comment.
Что-то цифр после запятой многовато у среднеквадратичного отклонения. Речь про случайную величину всё-таки, и её можно измерить только с некоторой погрешностью, так что выводить просто результат арифметических подсчётов бессмысленно — цифры есть, но буквально ничего не значат. Тут, наверное, вообще только целая часть имеет значение, ну, может, ещё одна цифра после запятой, для маленьких матриц
No description provided.