Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Седова Ольга. Задача 2. Вариант 16. Ленточная вертикальная схема умножения матрицы на вектор #386

Merged

Conversation

Sedova-Olga
Copy link
Contributor

@Sedova-Olga Sedova-Olga commented Nov 28, 2024

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

  1. Структура данных taskData:
    Существует некоторая структура данных taskData, которая содержит входные данные (матрицу и вектор) и выходные данные (результирующий вектор). Она предоставляет указатели на данные (taskData->inputs[0], taskData->inputs[1], taskData->outputs[0]) и их размеры (taskData->inputs_count[0], taskData->inputs_count[1]).
  2. Функция validation():
    Эта функция проверяет корректность входных данных. Она проверяет, что taskData не является нулевым указателем, что указатели на входные данные не являются нулевыми и что количество элементов в матрице кратно количеству элементов в векторе (что является необходимым условием для умножения матрицы на вектор).
  3. Класс ParallelMPI:
    Этот класс реализует параллельную версию алгоритма с использованием библиотеки Boost.MPI.
    -pre_processing(): Функция выполняет предварительную обработку данных. Она извлекает данные из taskData, вычисляет размеры матрицы (количество строк rows_ и столбцов cols_), распределяет работу между процессами (создаются векторы proc и off, которые определяют количество вычислений и смещение для каждого процесса). Распределение основано на вертикальном разбиении матрицы.
    -run(): Функция выполняет параллельное вычисление. Она использует функцию для рассылки (broadcast) данных всем процессам (размеры матрицы, распределение работы, входные данные). Каждый процесс вычисляет свою часть произведения, используя proc и off для определения диапазона вычислений. Результаты частичных вычислений сводятся с помощью функции reduce.
    -post_processing(): Функция выполняет обработку после вычислений. Если процесс имеет ранг 0 (главный процесс), то он собирает результаты частичных вычислений в результирующий вектор и записывает его в taskData->outputs[0].
  4. Класс SequentialMPI:
    Этот класс реализует последовательную версию алгоритма.
    -pre_processing(): Извлекает данные из taskData и вычисляет размеры матрицы.
    -run(): Выполняет последовательное вычисление произведения матрицы на вектор по ленточной вертикальной схеме, используя стандартные циклы.
    -post_processing(): Записывает результирующий вектор в taskData->outputs[0].

Используемые материалы при написании кода

  1. Гергель В.П. Введение в методы параллельного программирования Раздел 7 Параллельные методы умножения матрицы на вектор
  2. https://intuit.ru/studies/courses/1156/190/lecture/4952

@Sedova-Olga Sedova-Olga requested a review from venn2713 December 10, 2024 10:18
Comment on lines +109 to +110
boost::mpi::broadcast(world, input_matrix_1, 0);
boost::mpi::broadcast(world, input_vector_1, 0);
Copy link
Contributor

@kmichaelk kmichaelk Dec 10, 2024

Choose a reason for hiding this comment

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

Sending the entire matrix A to all processes is not efficient

You said that you've fixed it, but you did not

@Sedova-Olga
Copy link
Contributor Author

Sedova-Olga commented Dec 10, 2024

@allnes комментарии Сергея исправлены, он их проверил и поставил апрув

Comment on lines +10 to +44
TEST(sedova_o_vertical_ribbon_scheme_mpi, distribution1) {
int rows_ = 5;
int cols_ = 3;
int count_proc = 5;
std::vector<int> proc_(count_proc, 0);
std::vector<int> off(count_proc, 0);
if (count_proc > rows_) {
for (int i = 0; i < rows_; ++i) {
off[i] = i * cols_;
proc_[i] = cols_;
}
for (int i = rows_; i < count_proc; ++i) {
off[i] = -1;
proc_[i] = 0;
}
} else {
int count_proc_ = rows_ / count_proc;
int surplus = rows_ % count_proc;
int offset = 0;
for (int i = 0; i < count_proc; ++i) {
if (surplus > 0) {
proc_[i] = (count_proc_ + 1) * cols_;
--surplus;
} else {
proc_[i] = count_proc_ * cols_;
}
off[i] = offset;
offset += proc_[i];
}
}
std::vector<int> expected_proc = {3, 3, 3, 3, 3};
std::vector<int> expected_off = {0, 3, 6, 9, 12};
EXPECT_EQ(proc_, expected_proc);
EXPECT_EQ(off, expected_off);
}
Copy link
Contributor

@kmichaelk kmichaelk Dec 10, 2024

Choose a reason for hiding this comment

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

What are you testing in those tests? If you want to test the distribution resolving functionality from the Task class, then test it, testing a copy-pasted code does not guarantee that the actual code you use in Task is still working correctly as it may've been changed.

Copy link
Contributor

@kmichaelk kmichaelk left a comment

Choose a reason for hiding this comment

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

Where are the tests with random matrices?

@kmichaelk
Copy link
Contributor

kmichaelk commented Dec 10, 2024

Do not merge

I cloned your branch, ran perf tests as they do not run as part of actions for pull requests and encountered segfault on 3 processes.

So, with func tests it will be the same, that's why you didn't add random tests.

@m1likus
Copy link
Contributor

m1likus commented Dec 10, 2024

image

#include "core/perf/include/perf.hpp"
#include "mpi/sedova_o_vertical_ribbon_scheme/include/ops_mpi.hpp"

TEST(sedova_o_vertical_ribbon_scheme_mpi, Performance_Pipeline_Run) {
Copy link
Member

Choose a reason for hiding this comment

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

Perf tests should be renamed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@aobolensk aobolensk merged commit c82fba8 into learning-process:master Dec 15, 2024
11 checks passed
@venn2713
Copy link
Contributor

Боже, как я опять тут фигурирую... Ребят, прошу вас, свои смелые предположения формата "желтой прессы" можно и не оглашать на обильную аудиторию данного PR. Я вас несомненно понимаю, но не надо меня пожалуйста больше впутывать, с меня достаточно

@FiitWellwisher
Copy link

Вот вам человек сам и расписался, что задолбался участвовать в этом несчастном #386, по итогу имеем реально пустой набор тестов, устаревший аппрув от @KseniyaBeskhmelnova с единственной просьбой "add tests on big sizes of matrix" (где, кстати?) и такого же качества аппрув от @KolodkinGrigorii, если убрать предложенные им тесты, то реально останутся только тесты на краденную функцию для создания массивов в количестве 4 штук и тест на заранее заготовленные данные, который реально ничего не проверяет.

Почему нет requires > 4 reviews?

@kmichaelk
Copy link
Contributor

I am not happy with yet another participation in such a discussion with such a final again, but now I read your message and the code again and noticed one more thing - the only "real" test you wrote about actually tests the sequential version of the task (parallel version is named ParallelMPI), so all this task implementation is fiction that never worked

image

@kmichaelk
Copy link
Contributor

And post these tirades somewhere else next time (I hope this won't be needed though), it's very unpleasant for all of us to participate in your personal conflicts and we don't desire to involuntarily participate in

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

Successfully merging this pull request may close these issues.