From 1bf578c44213dc84f7cd71ce6063e539bbe98d28 Mon Sep 17 00:00:00 2001 From: Grayson Gall Date: Tue, 6 Aug 2024 11:32:41 -0600 Subject: [PATCH 1/2] Changing types to help with comparison of counters In the MultiPeriodAverager object there was some equality comparison between two floats of variables that should have been unsigned ints. While unable to replicate any reported failures I believe the issue was that on some of the testing machines this equality was never satisfied due to floating point equality comparison. addresses #253 --- include/postprocessors/MultiPeriodAverager.h | 12 +++--- .../PeriodicTimeIntegratedPostprocessor.h | 5 ++- src/postprocessors/MultiPeriodAverager.C | 37 ++++++++++--------- .../PeriodicTimeIntegratedPostprocessor.C | 12 +++--- 4 files changed, 35 insertions(+), 31 deletions(-) diff --git a/include/postprocessors/MultiPeriodAverager.h b/include/postprocessors/MultiPeriodAverager.h index 9b198bb8f5c..0a4436ce6ab 100644 --- a/include/postprocessors/MultiPeriodAverager.h +++ b/include/postprocessors/MultiPeriodAverager.h @@ -29,16 +29,16 @@ class MultiPeriodAverager : public GeneralPostprocessor Real _value; /// Where we are going to be storing the next average while we calculate it Real _temp_value; - /// The number of periods that have passed - Real _period_count; /// The time when the next period starts Real _next_period_start; - /// The counter for how many periods have passed since we last updated - Real _cyclic_period_count; /// inverse of the frequency const Real _period; - /// the number of periods over which we are averaging - Real _num_periods; /// The previous post process value of the post processor we are averaging over several periods const PostprocessorValue & _pps_value_old; + /// The number of periods that have passed + uint _period_count; + /// The counter for how many periods have passed since we last updated + uint _cyclic_period_count; + /// the number of periods over which we are averaging + uint _num_periods; }; diff --git a/include/postprocessors/PeriodicTimeIntegratedPostprocessor.h b/include/postprocessors/PeriodicTimeIntegratedPostprocessor.h index 98d13fb8bb8..c591d1cb7ad 100644 --- a/include/postprocessors/PeriodicTimeIntegratedPostprocessor.h +++ b/include/postprocessors/PeriodicTimeIntegratedPostprocessor.h @@ -22,7 +22,10 @@ class PeriodicTimeIntegratedPostprocessor : public MultipliedTimeIntegratedPostp virtual void execute() override; protected: + /// the period of time over which to integrate const Real _period; - Real _period_count; + /// the total number of periods that have occured + uint _period_count; + /// the point in the time when the next period begins Real _next_period_start; }; diff --git a/src/postprocessors/MultiPeriodAverager.C b/src/postprocessors/MultiPeriodAverager.C index d191fd02ec8..261aa73dd9a 100644 --- a/src/postprocessors/MultiPeriodAverager.C +++ b/src/postprocessors/MultiPeriodAverager.C @@ -17,7 +17,7 @@ MultiPeriodAverager::validParams() InputParameters params = GeneralPostprocessor::validParams(); params.addClassDescription( "Calculate the average value of a post processor over multiple periods"); - params.addRangeCheckedParam("number_of_periods", + params.addRangeCheckedParam("number_of_periods", "number_of_periods > 0", "The number of periods over which you are averaging"); params.addParam( @@ -33,10 +33,11 @@ MultiPeriodAverager::MultiPeriodAverager(const InputParameters & parameters) : GeneralPostprocessor(parameters), _value(0), _temp_value(0), - _period_count(0), _period(1.0 / getParam("cycle_frequency")), - _num_periods(getParam("number_of_periods")), - _pps_value_old(getPostprocessorValueOld("value")) + _pps_value_old(getPostprocessorValueOld("value")), + _period_count(0), + _cyclic_period_count(0), + _num_periods(getParam("number_of_periods")) { _next_period_start = _period; } @@ -46,21 +47,21 @@ MultiPeriodAverager::execute() { // lets check if we will be reaching the next period on the next // time step - if ((_t + _dt - _next_period_start) / _next_period_start >= 1e-6) - { - _period_count += 1; - _cyclic_period_count += 1; - _next_period_start = (_period_count + 1) * _period; - _temp_value += _pps_value_old / _num_periods; + if ((_t + _dt - _next_period_start) / _next_period_start < 1e-6) + return; + + _period_count += 1; + _cyclic_period_count += 1; + _next_period_start = (_period_count + 1) * _period; + _temp_value += _pps_value_old / _num_periods; + + /// if its time to update the average reset the temporary values + if (_cyclic_period_count != _num_periods) + return; - /// if its time to update the average reset the temporary values - if (_cyclic_period_count == _num_periods) - { - _value = _temp_value; - _cyclic_period_count = 0; - _temp_value = 0; - } - } + _value = _temp_value; + _cyclic_period_count = 0; + _temp_value = 0; } Real diff --git a/src/postprocessors/PeriodicTimeIntegratedPostprocessor.C b/src/postprocessors/PeriodicTimeIntegratedPostprocessor.C index 1651c0f9b60..127a966a150 100644 --- a/src/postprocessors/PeriodicTimeIntegratedPostprocessor.C +++ b/src/postprocessors/PeriodicTimeIntegratedPostprocessor.C @@ -41,10 +41,10 @@ PeriodicTimeIntegratedPostprocessor::execute() MultipliedTimeIntegratedPostprocessor::execute(); // lets check if we will be reaching the next period on the next // time step - if ((_t + _dt - _next_period_start) / _next_period_start >= 1e-6) - { - _period_count++; - _next_period_start = (_period_count + 1) * _period; - this->_value = 0; - } + if ((_t + _dt - _next_period_start) / _next_period_start < 1e-6) + return; + + _period_count++; + _next_period_start = (_period_count + 1) * _period; + this->_value = 0; } From fa2a85944e16101938a269b470b1a0436498b400 Mon Sep 17 00:00:00 2001 From: Grayson Gall <66559200+gsgall@users.noreply.github.com> Date: Tue, 6 Aug 2024 12:39:45 -0600 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Alex Lindsay --- include/postprocessors/MultiPeriodAverager.h | 6 +++--- .../postprocessors/PeriodicTimeIntegratedPostprocessor.h | 2 +- src/postprocessors/MultiPeriodAverager.C | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/postprocessors/MultiPeriodAverager.h b/include/postprocessors/MultiPeriodAverager.h index 0a4436ce6ab..28dd13df806 100644 --- a/include/postprocessors/MultiPeriodAverager.h +++ b/include/postprocessors/MultiPeriodAverager.h @@ -36,9 +36,9 @@ class MultiPeriodAverager : public GeneralPostprocessor /// The previous post process value of the post processor we are averaging over several periods const PostprocessorValue & _pps_value_old; /// The number of periods that have passed - uint _period_count; + unsigned int _period_count; /// The counter for how many periods have passed since we last updated - uint _cyclic_period_count; + unsigned int _cyclic_period_count; /// the number of periods over which we are averaging - uint _num_periods; + unsigned int _num_periods; }; diff --git a/include/postprocessors/PeriodicTimeIntegratedPostprocessor.h b/include/postprocessors/PeriodicTimeIntegratedPostprocessor.h index c591d1cb7ad..c8a9c0196d7 100644 --- a/include/postprocessors/PeriodicTimeIntegratedPostprocessor.h +++ b/include/postprocessors/PeriodicTimeIntegratedPostprocessor.h @@ -25,7 +25,7 @@ class PeriodicTimeIntegratedPostprocessor : public MultipliedTimeIntegratedPostp /// the period of time over which to integrate const Real _period; /// the total number of periods that have occured - uint _period_count; + unsigned int _period_count; /// the point in the time when the next period begins Real _next_period_start; }; diff --git a/src/postprocessors/MultiPeriodAverager.C b/src/postprocessors/MultiPeriodAverager.C index 261aa73dd9a..d1773cc0035 100644 --- a/src/postprocessors/MultiPeriodAverager.C +++ b/src/postprocessors/MultiPeriodAverager.C @@ -17,9 +17,9 @@ MultiPeriodAverager::validParams() InputParameters params = GeneralPostprocessor::validParams(); params.addClassDescription( "Calculate the average value of a post processor over multiple periods"); - params.addRangeCheckedParam("number_of_periods", - "number_of_periods > 0", - "The number of periods over which you are averaging"); + params.addRangeCheckedParam("number_of_periods", + "number_of_periods > 0", + "The number of periods over which you are averaging"); params.addParam( "value", "The name of the postprocessor you would like to average over multiple periods"); params.addRequiredRangeCheckedParam(