From ead1309f2e93fdc124b8830dcc6b0e27d77da4d6 Mon Sep 17 00:00:00 2001 From: John Tramm Date: Mon, 6 Jan 2020 21:15:41 +0000 Subject: [PATCH] fixed undefined behaviour bug. Issue was with the global tally accumulators. Specifically, I had changed it so that the global values get updated atomically once at particle death. However, in the old version, each thread dumps its threadprivate accumulator in parallel in a critical section at power iteration finalization. This was still being done in parallel, even though the global values were complete, so the resulting global tallies were getting recorded nthreads times. Making that region serial fixed it. --- src/simulation.cpp | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/simulation.cpp b/src/simulation.cpp index 230e16c17..e137f70bf 100644 --- a/src/simulation.cpp +++ b/src/simulation.cpp @@ -914,6 +914,7 @@ int openmc_next_batch(int* status) // ==================================================================== // LOOP OVER PARTICLES + /* #pragma omp parallel for schedule(runtime) for (int64_t i_work = 1; i_work <= simulation::work_per_rank; ++i_work) { // grab source particle from bank @@ -923,6 +924,7 @@ int openmc_next_batch(int* status) // transport particle p.transport(); } + */ /* #pragma omp parallel for schedule(runtime) for (int64_t i_work = 1; i_work <= simulation::work_per_rank; ++i_work) { @@ -935,7 +937,7 @@ int openmc_next_batch(int* status) } */ - //transport(); + transport(); // Accumulate time for transport simulation::time_transport.stop(); @@ -1175,27 +1177,20 @@ void finalize_generation() auto& gt = simulation::global_tallies; // Update global tallies with the omp private accumulation variables - #pragma omp parallel - { - #pragma omp critical(increment_global_tallies) - { - if (settings::run_mode == RUN_MODE_EIGENVALUE) { - gt(K_COLLISION, RESULT_VALUE) += global_tally_collision; - gt(K_ABSORPTION, RESULT_VALUE) += global_tally_absorption; - gt(K_TRACKLENGTH, RESULT_VALUE) += global_tally_tracklength; - } - gt(LEAKAGE, RESULT_VALUE) += global_tally_leakage; - } - - // reset threadprivate tallies - if (settings::run_mode == RUN_MODE_EIGENVALUE) { - global_tally_collision = 0.0; - global_tally_absorption = 0.0; - global_tally_tracklength = 0.0; - } - global_tally_leakage = 0.0; + if (settings::run_mode == RUN_MODE_EIGENVALUE) { + gt(K_COLLISION, RESULT_VALUE) += global_tally_collision; + gt(K_ABSORPTION, RESULT_VALUE) += global_tally_absorption; + gt(K_TRACKLENGTH, RESULT_VALUE) += global_tally_tracklength; } + gt(LEAKAGE, RESULT_VALUE) += global_tally_leakage; + // reset tallies + if (settings::run_mode == RUN_MODE_EIGENVALUE) { + global_tally_collision = 0.0; + global_tally_absorption = 0.0; + global_tally_tracklength = 0.0; + } + global_tally_leakage = 0.0; if (settings::run_mode == RUN_MODE_EIGENVALUE) { // We need to move all the stuff from the shared_fission_bank into the real one.