Skip to content

Добавила решение задачи#1

Open
Nigma-Ks wants to merge 7 commits intomainfrom
hw1-1
Open

Добавила решение задачи#1
Nigma-Ks wants to merge 7 commits intomainfrom
hw1-1

Conversation

@Nigma-Ks
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown

@ArtyomKopan ArtyomKopan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Пока что 1 балл из 4


public static class ComparisonMatrixCountingSpeeds
{
public static (double, double)[] Experiments(int sizeOfData, int amountOfTestsInSample) //size of data, mat ozgidanie, dispersiia
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

комментарии транслитом? Ты серьёзно?))

for (int i = 0; i < time.Count; i++)
{
double difference = time[i] - mathematicalExpectation;
sumOfSquareDifference += difference * difference;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно написать короче:
var sumOfSquareDifference = Enumerable.Range(0, time.Count).Sum(i => (time[i] - mathematicalExpectation) * (time[i] - mathematicalExpectation));

public int[,] matrixOfNumbers { get; private set; }
public int Height { get; private set; } = 0;
public int Width { get; private set; } = 0;
public Matrix(string filePath)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше передавать в конструктор сразу ширину, высоту и данные. Так положено по правилам инкапсуляции. Вдруг пользователь захочет вводить из консоли?

var matrix3Numbers = new int[matrix1.Height, matrix2.Width];

var matrixMyltiplyingThreads = new Thread[4];
for (byte i = 0; i < matrixMyltiplyingThreads.Length; i++)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а зачем тут byte? Мы же не на Си пишем :)

}
var matrix3Numbers = new int[matrix1.Height, matrix2.Width];

var matrixMyltiplyingThreads = new Thread[4];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а что, если программа будет запускаться на сервере со 128 процессорами? 4 потока -- это как-то маловато. Лучше написать var matrixMyltiplyingThreads = new Thread[Environment.ProcessorCount];

matrix3Numbers[currentLine, currentColumn] = 0;
for (int k = 0; k < matrix1.Width; k++)
{
matrix3Numbers[currentLine, currentColumn] += matrix1.matrixOfNumbers[currentLine, k] * matrix2.matrixOfNumbers[k, currentColumn];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно короче написать:
matrix3Numbers[currentLine, currentColumn] = Enumerable.Range(0, matrix1.Width).Sum(k => matrix1.matrixOfNumbers[currentLine, k] * matrix2.matrixOfNumbers[k, currentColumn]);

uint amountOfElementsInMatrix = (uint)matrix1.Height * (uint)matrix2.Width;
matrixMyltiplyingThreads[i] = new Thread(() =>
{
uint currentElement = (uint)numberOfStartElement;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно сделать проще:
var rowsPerThread = Math.Ceiling(matrix2.Height / Environment.ProcessorCount); и умножать по стольку строк в каждом потоке

});
}

foreach (var tread in matrixMyltiplyingThreads)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

по-английски пишется thread.
tread -- это "шагать"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

было бы неплохо добавить возможность вводить матрицы из консоли

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

кажется, у тебя что-то не то с результатами. Как при большой матрице параллельное умножение может работать медленнее последовательного?

Comment on lines +5 to +6
Matrix matrix1;
Matrix matrix2;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Matrix matrix1;
Matrix matrix2;
private Matrix matrix1;
private Matrix matrix2;

В .NET по традиции всегда явно пишут модификаторы видимости, даже если умолчания устраивают

[Test]
public void ExceptionsTest()
{
Assert.Throws<ArgumentNullException>(() => new Matrix(null));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут nullability-анализ ругается, если хотите null, напишите null!. Надо, чтобы компилировалось без предупреждений.


using System.Diagnostics;

public static class ComparisonMatrixCountingSpeeds
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Надо комментарии ко всем типам и всем public-членам

return new (double, double)[] { (mathematicalExpectationByThreads.Value, dispersionByThreads), (mathematicalExpectationSequential.Value, dispersionSequential) };
}

private static double DispersionCalculator(List<long> time, double mathematicalExpectation)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Методы должны называться глаголами в повелительной форме

double? mathematicalExpectationSequential = timeSequential.Average();
double dispersionByThreads = DispersionCalculator(timeByThreads, mathematicalExpectationByThreads.Value);
double dispersionSequential = DispersionCalculator(timeSequential, mathematicalExpectationSequential.Value);
return new (double, double)[] { (mathematicalExpectationByThreads.Value, dispersionByThreads), (mathematicalExpectationSequential.Value, dispersionSequential) };
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вместо массива кортежей, где всегда должно быть два элемента, сделали бы специальный struct. Явные именованные поля лучше соглашений

Comment on lines +96 to +99
if (matrix1 == null)
{
throw new ArgumentNullException(nameof(matrix1));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (matrix1 == null)
{
throw new ArgumentNullException(nameof(matrix1));
}
ArgumentNullException.ThrowIfNull(matrix1);

for (byte i = 0; i < matrixMyltiplyingThreads.Length; i++)
{
int numberOfStartElement = i;
uint amountOfElementsInMatrix = (uint)matrix1.Height * (uint)matrix2.Width;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Всё так, хотя и некритично. Лишние аннотации типов в целом не мешают

Comment on lines +147 to +148
int amountOfThreads = Environment.ProcessorCount;
var matrixMyltiplyingThreads = new Thread[amountOfThreads];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так может получиться больше потоков, чем нужно. В матрице может быть меньше Environment.ProcessorCount строк

FileStream fs = File.Create(matrix3FilePath);
if (!File.Exists(matrix3FilePath))
{
throw new ArgumentException("Файл не был создан");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В main нет смысла кидать исключения. потому что их даже в теории некому ловить. Выводите ошибку на консоль и всё.

Потоки: Последовательно:
Размер матрицы: 200*200
Мат ожидание: 231 150
Дисперсия: 1026 135
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Дисперсия не нужна, нужно среднеквадратичное отклонение

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants