Skip to content

Commit 6145491

Browse files
authored
Fix bounds assertion in GetBoxIndex; refactor SolveDirectionThomas; fix license header (#19)
* fix license header declaration in utils_aux.cc file * refactor: replace lambda dispatch with array lookup in the SolveDirectionThomas and used .at() for bound safety purposes * add bounds assertion to GetBoxIndex * test:add valid index and death test for GetBoxIndex change * add: private-to-public property conversion in diffusion_thomas_algorithm header file for test validation execution purposes * address clang-tidy warnings: add missing <cassert> and <array> headers * fix: apply clang-format-18 formatting corrections * executed git clang format command to new unit test file
1 parent 0e58af0 commit 6145491

File tree

4 files changed

+61
-45
lines changed

4 files changed

+61
-45
lines changed

src/diffusion_thomas_algorithm.cc

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
#include "core/param/param.h"
3030
#include "core/real_t.h"
3131
#include "core/resource_manager.h"
32+
#include <array>
33+
#include <cassert>
3234
#include <cstddef>
3335
#include <cstdint>
3436
#include <string>
@@ -173,6 +175,10 @@ void DiffusionThomasAlgorithm::SetConcentration(size_t idx, real_t amount) {
173175
// Flattens the 3D coordinates (x, y, z) into a 1D index
174176
size_t DiffusionThomasAlgorithm::GetBoxIndex(size_t x, size_t y,
175177
size_t z) const {
178+
assert(static_cast<int>(x) < resolution_ &&
179+
static_cast<int>(y) < resolution_ &&
180+
static_cast<int>(z) < resolution_ &&
181+
"GetBoxIndex: coordinate out of bounds");
176182
return z * resolution_ * resolution_ + y * resolution_ + x;
177183
}
178184

@@ -218,38 +224,17 @@ void DiffusionThomasAlgorithm::ApplyBoundaryConditionsIfNeeded() {
218224
}
219225

220226
void DiffusionThomasAlgorithm::SolveDirectionThomas(int direction) {
221-
const auto& thomas_denom = [this, direction]() -> const std::vector<real_t>& {
222-
if (direction == 0) {
223-
return thomas_denom_x_;
224-
}
225-
if (direction == 1) {
226-
return thomas_denom_y_;
227-
}
228-
// direction == 2
229-
return thomas_denom_z_;
230-
}();
227+
const std::array<const std::vector<real_t>*, 3> all_denoms = {
228+
&thomas_denom_x_, &thomas_denom_y_, &thomas_denom_z_};
231229

232-
const auto& thomas_c = [this, direction]() -> const std::vector<real_t>& {
233-
if (direction == 0) {
234-
return thomas_c_x_;
235-
}
236-
if (direction == 1) {
237-
return thomas_c_y_;
238-
}
239-
// direction == 2
240-
return thomas_c_z_;
241-
}();
230+
const std::array<const std::vector<real_t>*, 3> all_c = {
231+
&thomas_c_x_, &thomas_c_y_, &thomas_c_z_};
242232

243-
const int jump = [this, direction]() -> int {
244-
if (direction == 0) {
245-
return jump_i_;
246-
}
247-
if (direction == 1) {
248-
return jump_j_;
249-
}
250-
// direction == 2
251-
return jump_;
252-
}();
233+
const std::array<int, 3> all_jumps = {jump_i_, jump_j_, jump_};
234+
235+
const std::vector<real_t>& thomas_denom = *all_denoms.at(direction);
236+
const std::vector<real_t>& thomas_c = *all_c.at(direction);
237+
const int jump = all_jumps.at(direction);
253238

254239
#pragma omp parallel for collapse(2)
255240
for (int outer = 0; outer < resolution_; outer++) {

src/diffusion_thomas_algorithm.h

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,14 @@ class DiffusionThomasAlgorithm : public DiffusionGrid {
121121
/// always after the diffusion module
122122
void ComputeConsumptionsSecretions();
123123

124+
/// Convert 3D coordinates to linear index
125+
///
126+
/// @param x X-coordinate in voxel space
127+
/// @param y Y-coordinate in voxel space
128+
/// @param z Z-coordinate in voxel space
129+
/// @return Linear index in the flattened 3D array
130+
size_t GetBoxIndex(size_t x, size_t y, size_t z) const;
131+
124132
private:
125133
/// Number of voxels in each spatial direction
126134
int resolution_;
@@ -189,14 +197,6 @@ class DiffusionThomasAlgorithm : public DiffusionGrid {
189197
/// maintaining constant values at the grid boundaries.
190198
void ApplyDirichletBoundaryConditions();
191199

192-
/// Convert 3D coordinates to linear index
193-
///
194-
/// @param x X-coordinate in voxel space
195-
/// @param y Y-coordinate in voxel space
196-
/// @param z Z-coordinate in voxel space
197-
/// @return Linear index in the flattened 3D array
198-
size_t GetBoxIndex(size_t x, size_t y, size_t z) const;
199-
200200
/// Apply boundary conditions if Dirichlet boundaries are enabled
201201
void ApplyBoundaryConditionsIfNeeded();
202202

src/utils_aux.cc

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,7 @@
33
* Melina Luque
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
6-
* you may not use this file e // Count total cells, tumor cells of each
7-
type and tumor radius size_t total_num_tumor_cells = 0; size_t
8-
num_tumor_cells_type1 = 0; size_t num_tumor_cells_type2 = 0; size_t
9-
num_tumor_cells_type3 = 0; size_t num_tumor_cells_type4 = 0; size_t
10-
num_tumor_cells_type5_dead = 0; size_t num_alive_cart = 0; real_t tumor_radius
11-
= 0.0; real_t average_oncoprotein = 0.0; real_t average_oxygen_cancer_cells =
12-
0.0;in compliance with the License.
6+
* you may not use this file except in compliance with the License.
137
* You may obtain a copy of the License at
148
*
159
* http://www.apache.org/licenses/LICENSE-2.0

test/test-diffusion.cc

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#include "biodynamo.h"
2+
#include "diffusion_thomas_algorithm.h"
3+
#include "hyperparams.h"
4+
#include <gtest/gtest.h>
5+
6+
#define TEST_NAME typeid(*this).name()
7+
8+
namespace bdm {
9+
class DiffusionThomasAlgorithmTest : public ::testing::Test {
10+
protected:
11+
void SetUp() override {
12+
sim_ = std::make_unique<Simulation>(TEST_NAME);
13+
auto params = std::make_unique<SimParam>();
14+
Param::RegisterParamGroup(params.release());
15+
}
16+
17+
void TearDown() override { sim_.reset(); }
18+
19+
std::unique_ptr<Simulation> sim_;
20+
};
21+
22+
// case: valid index computes correctly
23+
TEST_F(DiffusionThomasAlgorithmTest, GetBoxIndexValidCoordinates) {
24+
DiffusionThomasAlgorithm grid(0, "test", 1.0, 0.1, 10, 0.01, false);
25+
EXPECT_EQ(grid.GetBoxIndex(4, 3, 2), static_cast<size_t>(234));
26+
// z * res * res + y * res + x = 2*100 + 3*10 + 4 = 234
27+
}
28+
29+
// case: out of bounds triggers assertion
30+
#ifndef NDEBUG
31+
TEST_F(DiffusionThomasAlgorithmTest, GetBoxIndexOutOfBoundsDeath) {
32+
DiffusionThomasAlgorithm grid(0, "test", 1000.0, 0.1, 10, 0.01, false);
33+
EXPECT_DEATH(grid.GetBoxIndex(10, 0, 0), "out of bounds");
34+
}
35+
#endif
36+
37+
} // namespace bdm

0 commit comments

Comments
 (0)