From a3c4b009add1460e60ba234bd284ab97c372a1cc Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Thu, 13 Oct 2022 16:19:40 +0200 Subject: [PATCH 01/14] Modify parallelization for ParaView export (#285) --- .../paraview/vtk_diffusion_grid.cc | 116 ++++++++++++------ .../paraview/vtk_diffusion_grid.h | 4 +- 2 files changed, 81 insertions(+), 39 deletions(-) diff --git a/src/core/visualization/paraview/vtk_diffusion_grid.cc b/src/core/visualization/paraview/vtk_diffusion_grid.cc index 8e816877f..dd7c918fa 100644 --- a/src/core/visualization/paraview/vtk_diffusion_grid.cc +++ b/src/core/visualization/paraview/vtk_diffusion_grid.cc @@ -42,8 +42,10 @@ VtkDiffusionGrid::VtkDiffusionGrid(const std::string& name, if (param->export_visualization) { auto* tinfo = ThreadInfo::GetInstance(); data_.resize(tinfo->GetMaxThreads()); + piece_boxes_z_.resize(tinfo->GetMaxThreads()); } else { data_.resize(1); + piece_boxes_z_.resize(1); } #pragma omp parallel for schedule(static, 1) @@ -104,6 +106,10 @@ VtkDiffusionGrid::~VtkDiffusionGrid() { bool VtkDiffusionGrid::IsUsed() const { return used_; } // ----------------------------------------------------------------------------- +// ToDo (tobias): +// * remove debug code (print statements) +// * remove now unnecessary branching +// * add comments void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { used_ = true; @@ -147,41 +153,60 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { } #pragma omp parallel for schedule(static, 1) - for (uint64_t i = 0; i < num_pieces_; ++i) { + for (uint64_t i = 0; i < piece_boxes_z_.size(); ++i) { uint64_t piece_elements; auto* e = piece_extents_[i].data(); - if (i < num_pieces_ - 1) { - piece_elements = piece_boxes_z_ * xy_num_boxes; - data_[i]->SetDimensions(num_boxes[0], num_boxes[1], piece_boxes_z_); + if (i < piece_boxes_z_.size() - 1) { + piece_elements = piece_boxes_z_[i] * xy_num_boxes; + data_[i]->SetDimensions(num_boxes[0], num_boxes[1], piece_boxes_z_[i]); data_[i]->SetExtent(e[0], e[1], e[2], e[3], e[4], - e[4] + piece_boxes_z_ - 1); + e[4] + piece_boxes_z_[i] - 1); } else { - piece_elements = piece_boxes_z_last_ * xy_num_boxes; - data_[i]->SetDimensions(num_boxes[0], num_boxes[1], piece_boxes_z_last_); + piece_elements = piece_boxes_z_[i] * xy_num_boxes; + data_[i]->SetDimensions(num_boxes[0], num_boxes[1], piece_boxes_z_[i]); data_[i]->SetExtent(e[0], e[1], e[2], e[3], e[4], - e[4] + piece_boxes_z_last_ - 1); + e[4] + piece_boxes_z_[i] - 1); } - real_t piece_origin_z = origin_z + box_length * piece_boxes_z_ * i; + // Compute partial sum of boxes until index i + auto sum = + std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.begin() + i, 0); + real_t piece_origin_z = origin_z + box_length * sum; data_[i]->SetOrigin(origin_x, origin_y, piece_origin_z); data_[i]->SetSpacing(box_length, box_length, box_length); + // // Debug + // std::cout << "sum: " << sum << std::endl; + // std::cout << "piece_origin_z : " << piece_origin_z << std::endl; if (concentration_array_idx_ != -1) { auto* co_ptr = const_cast(grid->GetAllConcentrations()); auto elements = static_cast(piece_elements); auto* array = static_cast( data_[i]->GetPointData()->GetArray(concentration_array_idx_)); - if (i < num_pieces_ - 1) { - array->SetArray(co_ptr + (elements * i), elements, 1); + auto offset = sum * xy_num_boxes; + if (i < piece_boxes_z_.size() - 1) { + // // Debug + // std::cout << "Offset : " << offset << std::endl; + // std::cout << "Value (begin) : " << co_ptr[offset] << std::endl; + // std::cout << "Value (end): " << co_ptr[offset + elements - 1] + // << std::endl; + array->SetArray(co_ptr + offset, elements, 1); } else { - array->SetArray(co_ptr + total_boxes - elements, elements, 1); + // // Debug + // std::cout << "Offset : " << offset << std::endl; + // std::cout << "Value (begin) : " << co_ptr[offset] << std::endl; + // std::cout << "Value (end): " << co_ptr[offset + elements - 1] + // << std::endl; + array->SetArray(co_ptr + offset, elements, 1); } + // // Debug + // std::cout << "elements : " << elements << std::endl; } if (gradient_array_idx_ != -1) { auto gr_ptr = const_cast(grid->GetAllGradients()); auto elements = static_cast(piece_elements * 3); auto* array = static_cast( data_[i]->GetPointData()->GetArray(gradient_array_idx_)); - if (i < num_pieces_ - 1) { + if (i < piece_boxes_z_.size() - 1) { array->SetArray(gr_ptr + (elements * i), elements, 1); } else { array->SetArray(gr_ptr + total_boxes - elements, elements, 1); @@ -196,47 +221,66 @@ void VtkDiffusionGrid::WriteToFile(uint64_t step) const { auto filename_prefix = Concat(name_, "-", step); ParallelVtiWriter writer; - writer(sim->GetOutputDir(), filename_prefix, data_, num_pieces_, + writer(sim->GetOutputDir(), filename_prefix, data_, piece_boxes_z_.size(), whole_extent_, piece_extents_); } // ----------------------------------------------------------------------------- void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { - if (num_pieces_target == 1) { - piece_boxes_z_last_ = boxes_z; - piece_boxes_z_ = 1; - num_pieces_ = 1; - } else if (boxes_z <= num_pieces_target) { - piece_boxes_z_last_ = 2; - piece_boxes_z_ = 1; - num_pieces_ = std::max(1ULL, boxes_z - 1); - } else { - auto boxes_per_piece = static_cast(boxes_z) / num_pieces_target; - piece_boxes_z_ = static_cast(std::ceil(boxes_per_piece)); - num_pieces_ = boxes_z / piece_boxes_z_; - piece_boxes_z_last_ = boxes_z - (num_pieces_ - 1) * piece_boxes_z_; + auto boxes_per_piece = static_cast(boxes_z) / num_pieces_target; + auto min_slices = static_cast(std::floor(boxes_per_piece)); + auto leftover = boxes_z % num_pieces_target; + // // Debug print info of all variables + // std::cout << "boxes_z : " << boxes_z << std::endl; + // std::cout << "num_pieces_target : " << num_pieces_target << std::endl; + // std::cout << "boxes_per_piece : " << boxes_per_piece << std::endl; + // std::cout << "min_slices : " << min_slices << std::endl; + // std::cout << "leftover : " << leftover << std::endl; + // Distribute leftover slices + for (uint64_t i = 0; i < piece_boxes_z_.size(); ++i) { + piece_boxes_z_[i] = min_slices; + if (leftover > 0) { + piece_boxes_z_[i]++; + leftover--; + } + } + // Shorten vector by removing all tailing zeros + auto it = std::find(piece_boxes_z_.begin(), piece_boxes_z_.end(), 0); + if (it != piece_boxes_z_.end()) { + piece_boxes_z_.resize(std::distance(piece_boxes_z_.begin(), it)); } - assert(num_pieces_ > 0); - assert(piece_boxes_z_last_ >= 1); - assert((num_pieces_ - 1) * piece_boxes_z_ + piece_boxes_z_last_ == boxes_z); + + // Check dissection + auto sum = std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.end(), 0); + assert(sum == boxes_z); + // // Debug: Calculate sum of all entries + // std::cout << "Sum : " << sum << std::endl; + // std::cout << "boxes_z : " << boxes_z << std::endl; + + // // Debug print the vector + // std::cout << "piece_boxes_z_ : "; + // for (auto i : piece_boxes_z_) { + // std::cout << i << " "; + // } + // std::cout << std::endl; } // ----------------------------------------------------------------------------- void VtkDiffusionGrid::CalcPieceExtents( const std::array& num_boxes) { - piece_extents_.resize(num_pieces_); - if (num_pieces_ == 1) { + piece_extents_.resize(piece_boxes_z_.size()); + if (piece_boxes_z_.size() == 1) { piece_extents_[0] = whole_extent_; return; } - int c = piece_boxes_z_; + int c = piece_boxes_z_[0]; piece_extents_[0] = {{0, static_cast(num_boxes[0]) - 1, 0, static_cast(num_boxes[1]) - 1, 0, c}}; - for (uint64_t i = 1; i < num_pieces_ - 1; ++i) { + for (uint64_t i = 1; i < piece_boxes_z_.size() - 1; ++i) { piece_extents_[i] = {{0, static_cast(num_boxes[0]) - 1, 0, static_cast(num_boxes[1]) - 1, c, - c + static_cast(piece_boxes_z_)}}; - c += piece_boxes_z_; + c + static_cast(piece_boxes_z_[i])}}; + c += piece_boxes_z_[i]; } piece_extents_.back() = {{0, static_cast(num_boxes[0]) - 1, 0, static_cast(num_boxes[1]) - 1, c, diff --git a/src/core/visualization/paraview/vtk_diffusion_grid.h b/src/core/visualization/paraview/vtk_diffusion_grid.h index d63db9558..aea223c55 100644 --- a/src/core/visualization/paraview/vtk_diffusion_grid.h +++ b/src/core/visualization/paraview/vtk_diffusion_grid.h @@ -55,9 +55,7 @@ class VtkDiffusionGrid { // vtkImageData objects for parallel processing. // ParaView calls the partition pieces. - uint64_t num_pieces_; - uint64_t piece_boxes_z_; - uint64_t piece_boxes_z_last_; + std::vector piece_boxes_z_; std::array whole_extent_; std::vector> piece_extents_; From 72589a97c13b931152322344b2415e0e3f1dd6da Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Fri, 14 Oct 2022 18:40:04 +0200 Subject: [PATCH 02/14] Fix unit tests - e.g. small scale export --- .../paraview/vtk_diffusion_grid.cc | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/core/visualization/paraview/vtk_diffusion_grid.cc b/src/core/visualization/paraview/vtk_diffusion_grid.cc index dd7c918fa..71c8291dd 100644 --- a/src/core/visualization/paraview/vtk_diffusion_grid.cc +++ b/src/core/visualization/paraview/vtk_diffusion_grid.cc @@ -227,16 +227,28 @@ void VtkDiffusionGrid::WriteToFile(uint64_t step) const { // ----------------------------------------------------------------------------- void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { + // The following lines are used to reduce the number of pieces such that every + // .vti file contains at least 2 boxes in the z direction. For unknown reason, + // the .pvti file does not load correctly if we export 6 slices with 6 + // threads, e.g. write each slice to a separate .vti file. + if (num_pieces_target > boxes_z / 2 && num_pieces_target > 1) { + num_pieces_target = std::floor(static_cast(boxes_z) / 2); + } + piece_boxes_z_.resize(num_pieces_target); + + // Compute the number of boxes in each piece auto boxes_per_piece = static_cast(boxes_z) / num_pieces_target; auto min_slices = static_cast(std::floor(boxes_per_piece)); auto leftover = boxes_z % num_pieces_target; + // // Debug print info of all variables // std::cout << "boxes_z : " << boxes_z << std::endl; // std::cout << "num_pieces_target : " << num_pieces_target << std::endl; // std::cout << "boxes_per_piece : " << boxes_per_piece << std::endl; // std::cout << "min_slices : " << min_slices << std::endl; // std::cout << "leftover : " << leftover << std::endl; - // Distribute leftover slices + + // Distribute default slices and leftover slices for (uint64_t i = 0; i < piece_boxes_z_.size(); ++i) { piece_boxes_z_[i] = min_slices; if (leftover > 0) { @@ -244,11 +256,6 @@ void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { leftover--; } } - // Shorten vector by removing all tailing zeros - auto it = std::find(piece_boxes_z_.begin(), piece_boxes_z_.end(), 0); - if (it != piece_boxes_z_.end()) { - piece_boxes_z_.resize(std::distance(piece_boxes_z_.begin(), it)); - } // Check dissection auto sum = std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.end(), 0); From 644afb8ab4a1feb473cdbc5a295934b66de17a8c Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Fri, 14 Oct 2022 18:41:14 +0200 Subject: [PATCH 03/14] Remove debug code --- .../paraview/vtk_diffusion_grid.cc | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/src/core/visualization/paraview/vtk_diffusion_grid.cc b/src/core/visualization/paraview/vtk_diffusion_grid.cc index 71c8291dd..201db63af 100644 --- a/src/core/visualization/paraview/vtk_diffusion_grid.cc +++ b/src/core/visualization/paraview/vtk_diffusion_grid.cc @@ -173,9 +173,6 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { real_t piece_origin_z = origin_z + box_length * sum; data_[i]->SetOrigin(origin_x, origin_y, piece_origin_z); data_[i]->SetSpacing(box_length, box_length, box_length); - // // Debug - // std::cout << "sum: " << sum << std::endl; - // std::cout << "piece_origin_z : " << piece_origin_z << std::endl; if (concentration_array_idx_ != -1) { auto* co_ptr = const_cast(grid->GetAllConcentrations()); @@ -184,22 +181,10 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { data_[i]->GetPointData()->GetArray(concentration_array_idx_)); auto offset = sum * xy_num_boxes; if (i < piece_boxes_z_.size() - 1) { - // // Debug - // std::cout << "Offset : " << offset << std::endl; - // std::cout << "Value (begin) : " << co_ptr[offset] << std::endl; - // std::cout << "Value (end): " << co_ptr[offset + elements - 1] - // << std::endl; array->SetArray(co_ptr + offset, elements, 1); } else { - // // Debug - // std::cout << "Offset : " << offset << std::endl; - // std::cout << "Value (begin) : " << co_ptr[offset] << std::endl; - // std::cout << "Value (end): " << co_ptr[offset + elements - 1] - // << std::endl; array->SetArray(co_ptr + offset, elements, 1); } - // // Debug - // std::cout << "elements : " << elements << std::endl; } if (gradient_array_idx_ != -1) { auto gr_ptr = const_cast(grid->GetAllGradients()); @@ -241,13 +226,6 @@ void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { auto min_slices = static_cast(std::floor(boxes_per_piece)); auto leftover = boxes_z % num_pieces_target; - // // Debug print info of all variables - // std::cout << "boxes_z : " << boxes_z << std::endl; - // std::cout << "num_pieces_target : " << num_pieces_target << std::endl; - // std::cout << "boxes_per_piece : " << boxes_per_piece << std::endl; - // std::cout << "min_slices : " << min_slices << std::endl; - // std::cout << "leftover : " << leftover << std::endl; - // Distribute default slices and leftover slices for (uint64_t i = 0; i < piece_boxes_z_.size(); ++i) { piece_boxes_z_[i] = min_slices; @@ -260,16 +238,6 @@ void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { // Check dissection auto sum = std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.end(), 0); assert(sum == boxes_z); - // // Debug: Calculate sum of all entries - // std::cout << "Sum : " << sum << std::endl; - // std::cout << "boxes_z : " << boxes_z << std::endl; - - // // Debug print the vector - // std::cout << "piece_boxes_z_ : "; - // for (auto i : piece_boxes_z_) { - // std::cout << i << " "; - // } - // std::cout << std::endl; } // ----------------------------------------------------------------------------- From 6d36d19d8e62e8c91e720e28020715f9e1a30fa9 Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Fri, 14 Oct 2022 18:48:34 +0200 Subject: [PATCH 04/14] Remove now obsolete if statements --- .../paraview/vtk_diffusion_grid.cc | 25 ++++++------------- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/src/core/visualization/paraview/vtk_diffusion_grid.cc b/src/core/visualization/paraview/vtk_diffusion_grid.cc index 201db63af..802a4f75f 100644 --- a/src/core/visualization/paraview/vtk_diffusion_grid.cc +++ b/src/core/visualization/paraview/vtk_diffusion_grid.cc @@ -156,17 +156,10 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { for (uint64_t i = 0; i < piece_boxes_z_.size(); ++i) { uint64_t piece_elements; auto* e = piece_extents_[i].data(); - if (i < piece_boxes_z_.size() - 1) { - piece_elements = piece_boxes_z_[i] * xy_num_boxes; - data_[i]->SetDimensions(num_boxes[0], num_boxes[1], piece_boxes_z_[i]); - data_[i]->SetExtent(e[0], e[1], e[2], e[3], e[4], - e[4] + piece_boxes_z_[i] - 1); - } else { - piece_elements = piece_boxes_z_[i] * xy_num_boxes; - data_[i]->SetDimensions(num_boxes[0], num_boxes[1], piece_boxes_z_[i]); - data_[i]->SetExtent(e[0], e[1], e[2], e[3], e[4], - e[4] + piece_boxes_z_[i] - 1); - } + piece_elements = piece_boxes_z_[i] * xy_num_boxes; + data_[i]->SetDimensions(num_boxes[0], num_boxes[1], piece_boxes_z_[i]); + data_[i]->SetExtent(e[0], e[1], e[2], e[3], e[4], + e[4] + piece_boxes_z_[i] - 1); // Compute partial sum of boxes until index i auto sum = std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.begin() + i, 0); @@ -180,11 +173,7 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { auto* array = static_cast( data_[i]->GetPointData()->GetArray(concentration_array_idx_)); auto offset = sum * xy_num_boxes; - if (i < piece_boxes_z_.size() - 1) { - array->SetArray(co_ptr + offset, elements, 1); - } else { - array->SetArray(co_ptr + offset, elements, 1); - } + array->SetArray(co_ptr + offset, elements, 1); } if (gradient_array_idx_ != -1) { auto gr_ptr = const_cast(grid->GetAllGradients()); @@ -236,8 +225,8 @@ void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { } // Check dissection - auto sum = std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.end(), 0); - assert(sum == boxes_z); + assert(boxes_z == + std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.end(), 0)); } // ----------------------------------------------------------------------------- From 322a46bcee4e16d2c7b01be51abfbb76e04c2567 Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Fri, 14 Oct 2022 19:02:02 +0200 Subject: [PATCH 05/14] Add test that would fail on master with 4 or 8 threads --- .../visualization/paraview/integration_test.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/unit/core/visualization/paraview/integration_test.cc b/test/unit/core/visualization/paraview/integration_test.cc index 44f07f47c..2ef7d1bab 100644 --- a/test/unit/core/visualization/paraview/integration_test.cc +++ b/test/unit/core/visualization/paraview/integration_test.cc @@ -141,6 +141,20 @@ TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_SlicesGtNumThreads) { LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, max_threads)); } +// ----------------------------------------------------------------------------- +// We observed in #285 that the pvsm file is not created correctly for certain +// combinations of number of threads and number of slices. This test is to +// ensure that this does not happen again. +TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_PrimeNumberSlice) { + auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); + if (max_threads == 4 || max_threads == 8) { + LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, 19)); + } else { + Log::Info("ExportDiffusionGrid_PrimeNumberSlice", + "This test needs to be executed with 4 or 8 threads."); + } +} + // ----------------------------------------------------------------------------- TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGridLoadWithoutPVSM) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); From 357802bf347367dbb2f5d0dd842cbf91940604a2 Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Fri, 14 Oct 2022 19:15:12 +0200 Subject: [PATCH 06/14] Add a stress test to iterate the resolution Hope to find failure across supported platforms. Fails on master. --- .../core/visualization/paraview/integration_test.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/unit/core/visualization/paraview/integration_test.cc b/test/unit/core/visualization/paraview/integration_test.cc index 2ef7d1bab..d79e8584d 100644 --- a/test/unit/core/visualization/paraview/integration_test.cc +++ b/test/unit/core/visualization/paraview/integration_test.cc @@ -155,6 +155,17 @@ TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_PrimeNumberSlice) { } } +// ----------------------------------------------------------------------------- +// Stress test scanning different grid resolutions. Used to run through GHA +// once, disabled afterwards. Also related to #285. +TEST(FLAKY_ParaviewIntegrationTest, + ExportDiffusionGrid_ScanResolutionForFailure) { + auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); + for (size_t i = 3; i < 25; i++) { + LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, i)); + } +} + // ----------------------------------------------------------------------------- TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGridLoadWithoutPVSM) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); From b7d1a96fad7f899b9aa4a870261dd9afa349c1d9 Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Fri, 14 Oct 2022 19:18:48 +0200 Subject: [PATCH 07/14] Disable ScanResolutionForFailure for CI --- test/unit/core/visualization/paraview/integration_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/core/visualization/paraview/integration_test.cc b/test/unit/core/visualization/paraview/integration_test.cc index d79e8584d..fbc9f7eac 100644 --- a/test/unit/core/visualization/paraview/integration_test.cc +++ b/test/unit/core/visualization/paraview/integration_test.cc @@ -158,7 +158,7 @@ TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_PrimeNumberSlice) { // ----------------------------------------------------------------------------- // Stress test scanning different grid resolutions. Used to run through GHA // once, disabled afterwards. Also related to #285. -TEST(FLAKY_ParaviewIntegrationTest, +TEST(DISABLED_FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_ScanResolutionForFailure) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); for (size_t i = 3; i < 25; i++) { From 13e11e3b4c7468a082e5bc15dfff8748d501d4f8 Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Mon, 17 Oct 2022 10:23:09 +0200 Subject: [PATCH 08/14] Remove ToDo comment --- src/core/visualization/paraview/vtk_diffusion_grid.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/core/visualization/paraview/vtk_diffusion_grid.cc b/src/core/visualization/paraview/vtk_diffusion_grid.cc index 802a4f75f..1ff939b21 100644 --- a/src/core/visualization/paraview/vtk_diffusion_grid.cc +++ b/src/core/visualization/paraview/vtk_diffusion_grid.cc @@ -106,10 +106,6 @@ VtkDiffusionGrid::~VtkDiffusionGrid() { bool VtkDiffusionGrid::IsUsed() const { return used_; } // ----------------------------------------------------------------------------- -// ToDo (tobias): -// * remove debug code (print statements) -// * remove now unnecessary branching -// * add comments void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { used_ = true; From c351e764e7b44e6e147eb7090aa9332278210bcd Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Mon, 17 Oct 2022 10:39:14 +0200 Subject: [PATCH 09/14] Make uint to int conversion explicit --- .../visualization/paraview/vtk_diffusion_grid.cc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/core/visualization/paraview/vtk_diffusion_grid.cc b/src/core/visualization/paraview/vtk_diffusion_grid.cc index 1ff939b21..6fbb1db1b 100644 --- a/src/core/visualization/paraview/vtk_diffusion_grid.cc +++ b/src/core/visualization/paraview/vtk_diffusion_grid.cc @@ -153,9 +153,10 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { uint64_t piece_elements; auto* e = piece_extents_[i].data(); piece_elements = piece_boxes_z_[i] * xy_num_boxes; - data_[i]->SetDimensions(num_boxes[0], num_boxes[1], piece_boxes_z_[i]); + data_[i]->SetDimensions(num_boxes[0], num_boxes[1], + static_cast(piece_boxes_z_[i])); data_[i]->SetExtent(e[0], e[1], e[2], e[3], e[4], - e[4] + piece_boxes_z_[i] - 1); + e[4] + static_cast(piece_boxes_z_[i]) - 1); // Compute partial sum of boxes until index i auto sum = std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.begin() + i, 0); @@ -202,7 +203,8 @@ void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { // the .pvti file does not load correctly if we export 6 slices with 6 // threads, e.g. write each slice to a separate .vti file. if (num_pieces_target > boxes_z / 2 && num_pieces_target > 1) { - num_pieces_target = std::floor(static_cast(boxes_z) / 2); + num_pieces_target = + static_cast(std::floor(static_cast(boxes_z) / 2)); } piece_boxes_z_.resize(num_pieces_target); @@ -233,14 +235,14 @@ void VtkDiffusionGrid::CalcPieceExtents( piece_extents_[0] = whole_extent_; return; } - int c = piece_boxes_z_[0]; + int c = static_cast(piece_boxes_z_[0]); piece_extents_[0] = {{0, static_cast(num_boxes[0]) - 1, 0, static_cast(num_boxes[1]) - 1, 0, c}}; for (uint64_t i = 1; i < piece_boxes_z_.size() - 1; ++i) { piece_extents_[i] = {{0, static_cast(num_boxes[0]) - 1, 0, static_cast(num_boxes[1]) - 1, c, c + static_cast(piece_boxes_z_[i])}}; - c += piece_boxes_z_[i]; + c += static_cast(piece_boxes_z_[i]); } piece_extents_.back() = {{0, static_cast(num_boxes[0]) - 1, 0, static_cast(num_boxes[1]) - 1, c, From b38c92e8b6853a12df85e4d8553fe396930bb6ad Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Mon, 17 Oct 2022 11:30:28 +0200 Subject: [PATCH 10/14] Debug commit [do not merge] --- .../paraview/vtk_diffusion_grid.cc | 30 ++++++++++++++++++- .../paraview/integration_test.cc | 29 +++++++++++------- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/core/visualization/paraview/vtk_diffusion_grid.cc b/src/core/visualization/paraview/vtk_diffusion_grid.cc index 6fbb1db1b..c0e30c0d1 100644 --- a/src/core/visualization/paraview/vtk_diffusion_grid.cc +++ b/src/core/visualization/paraview/vtk_diffusion_grid.cc @@ -148,7 +148,7 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { return; } -#pragma omp parallel for schedule(static, 1) + // #pragma omp parallel for schedule(static, 1) for (uint64_t i = 0; i < piece_boxes_z_.size(); ++i) { uint64_t piece_elements; auto* e = piece_extents_[i].data(); @@ -163,6 +163,9 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { real_t piece_origin_z = origin_z + box_length * sum; data_[i]->SetOrigin(origin_x, origin_y, piece_origin_z); data_[i]->SetSpacing(box_length, box_length, box_length); + // Debug + std::cout << "sum: " << sum << std::endl; + std::cout << "piece_origin_z : " << piece_origin_z << std::endl; if (concentration_array_idx_ != -1) { auto* co_ptr = const_cast(grid->GetAllConcentrations()); @@ -170,6 +173,12 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { auto* array = static_cast( data_[i]->GetPointData()->GetArray(concentration_array_idx_)); auto offset = sum * xy_num_boxes; + // Debug + std::cout << "Offset : " << offset << std::endl; + std::cout << "elements : " << elements << std::endl; + std::cout << "Value (begin) : " << co_ptr[offset] << std::endl; + std::cout << "Value (end): " << co_ptr[offset + elements - 1] + << std::endl; array->SetArray(co_ptr + offset, elements, 1); } if (gradient_array_idx_ != -1) { @@ -213,6 +222,13 @@ void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { auto min_slices = static_cast(std::floor(boxes_per_piece)); auto leftover = boxes_z % num_pieces_target; + // Debug print info of all variables + std::cout << "boxes_z : " << boxes_z << std::endl; + std::cout << "num_pieces_target : " << num_pieces_target << std::endl; + std::cout << "boxes_per_piece : " << boxes_per_piece << std::endl; + std::cout << "min_slices : " << min_slices << std::endl; + std::cout << "leftover : " << leftover << std::endl; + // Distribute default slices and leftover slices for (uint64_t i = 0; i < piece_boxes_z_.size(); ++i) { piece_boxes_z_[i] = min_slices; @@ -225,6 +241,18 @@ void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { // Check dissection assert(boxes_z == std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.end(), 0)); + + // Debug: Calculate sum of all entries + auto sum = std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.end(), 0); + std::cout << "Sum : " << sum << std::endl; + std::cout << "boxes_z : " << boxes_z << std::endl; + + // Debug print the vector + std::cout << "piece_boxes_z_ : "; + for (auto i : piece_boxes_z_) { + std::cout << i << " "; + } + std::cout << std::endl; } // ----------------------------------------------------------------------------- diff --git a/test/unit/core/visualization/paraview/integration_test.cc b/test/unit/core/visualization/paraview/integration_test.cc index fbc9f7eac..3e0841b72 100644 --- a/test/unit/core/visualization/paraview/integration_test.cc +++ b/test/unit/core/visualization/paraview/integration_test.cc @@ -125,20 +125,24 @@ void RunDiffusionGridTest(uint64_t max_bound, uint64_t resolution, } ASSERT_TRUE(fs::exists(Concat(output_dir, "/valid"))); // Test will run in separate process. - exit(0); + // exit(0); } // ----------------------------------------------------------------------------- TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_SlicesLtNumThreads) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); - LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(std::max(max_threads - 1, 1), - std::max(max_threads - 1, 1))); + RunDiffusionGridTest(std::max(max_threads - 1, 1), + std::max(max_threads - 1, 1)); + // LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(std::max(max_threads - 1, 1), + // std::max(max_threads - 1, 1))); } // ----------------------------------------------------------------------------- TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_SlicesGtNumThreads) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); - LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, max_threads)); + RunDiffusionGridTest(3 * max_threads + 1, max_threads); + // LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, + // max_threads)); } // ----------------------------------------------------------------------------- @@ -147,11 +151,11 @@ TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_SlicesGtNumThreads) { // ensure that this does not happen again. TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_PrimeNumberSlice) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); - if (max_threads == 4 || max_threads == 8) { - LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, 19)); - } else { - Log::Info("ExportDiffusionGrid_PrimeNumberSlice", - "This test needs to be executed with 4 or 8 threads."); + RunDiffusionGridTest(3 * max_threads + 1, 19); + // LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, 19)); + if (!(max_threads == 4) || !(max_threads == 8)) { + Log::Warning("ExportDiffusionGrid_PrimeNumberSlice", + "This test needs to be executed with 4 or 8 threads."); } } @@ -169,8 +173,11 @@ TEST(DISABLED_FLAKY_ParaviewIntegrationTest, // ----------------------------------------------------------------------------- TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGridLoadWithoutPVSM) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); - LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest( - std::max(max_threads - 1, 1), std::max(max_threads - 1, 1), true, false)); + RunDiffusionGridTest(std::max(max_threads - 1, 1), + std::max(max_threads - 1, 1), true, false); + // LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest( + // std::max(max_threads - 1, 1), std::max(max_threads - 1, 1), true, + // false)); } // Insitu-visualization not supported on macOS. Thus, we do not run these test From d329e394bb11062e986672bf3494b4b5203086d2 Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Fri, 28 Oct 2022 12:07:05 +0100 Subject: [PATCH 11/14] Fix code smells --- src/core/visualization/paraview/vtk_diffusion_grid.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/visualization/paraview/vtk_diffusion_grid.cc b/src/core/visualization/paraview/vtk_diffusion_grid.cc index c0e30c0d1..beb70d78b 100644 --- a/src/core/visualization/paraview/vtk_diffusion_grid.cc +++ b/src/core/visualization/paraview/vtk_diffusion_grid.cc @@ -153,7 +153,8 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { uint64_t piece_elements; auto* e = piece_extents_[i].data(); piece_elements = piece_boxes_z_[i] * xy_num_boxes; - data_[i]->SetDimensions(num_boxes[0], num_boxes[1], + data_[i]->SetDimensions(static_cast(num_boxes[0]), + static_cast(num_boxes[1]), static_cast(piece_boxes_z_[i])); data_[i]->SetExtent(e[0], e[1], e[2], e[3], e[4], e[4] + static_cast(piece_boxes_z_[i]) - 1); @@ -263,7 +264,7 @@ void VtkDiffusionGrid::CalcPieceExtents( piece_extents_[0] = whole_extent_; return; } - int c = static_cast(piece_boxes_z_[0]); + auto c = static_cast(piece_boxes_z_[0]); piece_extents_[0] = {{0, static_cast(num_boxes[0]) - 1, 0, static_cast(num_boxes[1]) - 1, 0, c}}; for (uint64_t i = 1; i < piece_boxes_z_.size() - 1; ++i) { From 8a8f18874634b4d1e501a79fa968e815653aef2f Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Fri, 28 Oct 2022 14:29:21 +0100 Subject: [PATCH 12/14] Fix if statement logic --- test/unit/core/visualization/paraview/integration_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/core/visualization/paraview/integration_test.cc b/test/unit/core/visualization/paraview/integration_test.cc index 3e0841b72..719918ad6 100644 --- a/test/unit/core/visualization/paraview/integration_test.cc +++ b/test/unit/core/visualization/paraview/integration_test.cc @@ -153,7 +153,7 @@ TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_PrimeNumberSlice) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); RunDiffusionGridTest(3 * max_threads + 1, 19); // LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, 19)); - if (!(max_threads == 4) || !(max_threads == 8)) { + if (!(max_threads == 4 || max_threads == 8)) { Log::Warning("ExportDiffusionGrid_PrimeNumberSlice", "This test needs to be executed with 4 or 8 threads."); } From 91ec5412e10adb727143c6fd1b2bdf546e15f7dd Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Fri, 28 Oct 2022 14:38:17 +0100 Subject: [PATCH 13/14] Remove Debug output --- .../paraview/vtk_diffusion_grid.cc | 28 ------------------- .../paraview/integration_test.cc | 22 +++++---------- 2 files changed, 7 insertions(+), 43 deletions(-) diff --git a/src/core/visualization/paraview/vtk_diffusion_grid.cc b/src/core/visualization/paraview/vtk_diffusion_grid.cc index beb70d78b..d6d3736c1 100644 --- a/src/core/visualization/paraview/vtk_diffusion_grid.cc +++ b/src/core/visualization/paraview/vtk_diffusion_grid.cc @@ -164,9 +164,6 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { real_t piece_origin_z = origin_z + box_length * sum; data_[i]->SetOrigin(origin_x, origin_y, piece_origin_z); data_[i]->SetSpacing(box_length, box_length, box_length); - // Debug - std::cout << "sum: " << sum << std::endl; - std::cout << "piece_origin_z : " << piece_origin_z << std::endl; if (concentration_array_idx_ != -1) { auto* co_ptr = const_cast(grid->GetAllConcentrations()); @@ -174,12 +171,6 @@ void VtkDiffusionGrid::Update(const DiffusionGrid* grid) { auto* array = static_cast( data_[i]->GetPointData()->GetArray(concentration_array_idx_)); auto offset = sum * xy_num_boxes; - // Debug - std::cout << "Offset : " << offset << std::endl; - std::cout << "elements : " << elements << std::endl; - std::cout << "Value (begin) : " << co_ptr[offset] << std::endl; - std::cout << "Value (end): " << co_ptr[offset + elements - 1] - << std::endl; array->SetArray(co_ptr + offset, elements, 1); } if (gradient_array_idx_ != -1) { @@ -223,13 +214,6 @@ void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { auto min_slices = static_cast(std::floor(boxes_per_piece)); auto leftover = boxes_z % num_pieces_target; - // Debug print info of all variables - std::cout << "boxes_z : " << boxes_z << std::endl; - std::cout << "num_pieces_target : " << num_pieces_target << std::endl; - std::cout << "boxes_per_piece : " << boxes_per_piece << std::endl; - std::cout << "min_slices : " << min_slices << std::endl; - std::cout << "leftover : " << leftover << std::endl; - // Distribute default slices and leftover slices for (uint64_t i = 0; i < piece_boxes_z_.size(); ++i) { piece_boxes_z_[i] = min_slices; @@ -242,18 +226,6 @@ void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { // Check dissection assert(boxes_z == std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.end(), 0)); - - // Debug: Calculate sum of all entries - auto sum = std::accumulate(piece_boxes_z_.begin(), piece_boxes_z_.end(), 0); - std::cout << "Sum : " << sum << std::endl; - std::cout << "boxes_z : " << boxes_z << std::endl; - - // Debug print the vector - std::cout << "piece_boxes_z_ : "; - for (auto i : piece_boxes_z_) { - std::cout << i << " "; - } - std::cout << std::endl; } // ----------------------------------------------------------------------------- diff --git a/test/unit/core/visualization/paraview/integration_test.cc b/test/unit/core/visualization/paraview/integration_test.cc index 719918ad6..17f62ba62 100644 --- a/test/unit/core/visualization/paraview/integration_test.cc +++ b/test/unit/core/visualization/paraview/integration_test.cc @@ -125,24 +125,20 @@ void RunDiffusionGridTest(uint64_t max_bound, uint64_t resolution, } ASSERT_TRUE(fs::exists(Concat(output_dir, "/valid"))); // Test will run in separate process. - // exit(0); + exit(0); } // ----------------------------------------------------------------------------- TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_SlicesLtNumThreads) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); - RunDiffusionGridTest(std::max(max_threads - 1, 1), - std::max(max_threads - 1, 1)); - // LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(std::max(max_threads - 1, 1), - // std::max(max_threads - 1, 1))); + LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(std::max(max_threads - 1, 1), + std::max(max_threads - 1, 1))); } // ----------------------------------------------------------------------------- TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_SlicesGtNumThreads) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); - RunDiffusionGridTest(3 * max_threads + 1, max_threads); - // LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, - // max_threads)); + LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, max_threads)); } // ----------------------------------------------------------------------------- @@ -151,12 +147,11 @@ TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_SlicesGtNumThreads) { // ensure that this does not happen again. TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGrid_PrimeNumberSlice) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); - RunDiffusionGridTest(3 * max_threads + 1, 19); - // LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, 19)); if (!(max_threads == 4 || max_threads == 8)) { Log::Warning("ExportDiffusionGrid_PrimeNumberSlice", "This test needs to be executed with 4 or 8 threads."); } + LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest(3 * max_threads + 1, 19)); } // ----------------------------------------------------------------------------- @@ -173,11 +168,8 @@ TEST(DISABLED_FLAKY_ParaviewIntegrationTest, // ----------------------------------------------------------------------------- TEST(FLAKY_ParaviewIntegrationTest, ExportDiffusionGridLoadWithoutPVSM) { auto max_threads = ThreadInfo::GetInstance()->GetMaxThreads(); - RunDiffusionGridTest(std::max(max_threads - 1, 1), - std::max(max_threads - 1, 1), true, false); - // LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest( - // std::max(max_threads - 1, 1), std::max(max_threads - 1, 1), true, - // false)); + LAUNCH_IN_NEW_PROCESS(RunDiffusionGridTest( + std::max(max_threads - 1, 1), std::max(max_threads - 1, 1), true, false)); } // Insitu-visualization not supported on macOS. Thus, we do not run these test From 9c59af23d090839d63b2bec15b8d2fdf81249e1e Mon Sep 17 00:00:00 2001 From: Tobias Duswald Date: Mon, 31 Oct 2022 16:02:58 +0000 Subject: [PATCH 14/14] Fix for OMP_NUM_THREADS=2 case --- src/core/visualization/paraview/vtk_diffusion_grid.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/visualization/paraview/vtk_diffusion_grid.cc b/src/core/visualization/paraview/vtk_diffusion_grid.cc index d6d3736c1..74bf659a4 100644 --- a/src/core/visualization/paraview/vtk_diffusion_grid.cc +++ b/src/core/visualization/paraview/vtk_diffusion_grid.cc @@ -206,6 +206,7 @@ void VtkDiffusionGrid::Dissect(uint64_t boxes_z, uint64_t num_pieces_target) { if (num_pieces_target > boxes_z / 2 && num_pieces_target > 1) { num_pieces_target = static_cast(std::floor(static_cast(boxes_z) / 2)); + num_pieces_target += (num_pieces_target == 0) ? 1 : 0; } piece_boxes_z_.resize(num_pieces_target);