Averaging accumulator template with overflow detection/prevention

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
4
down vote

favorite












This is a class/template I just wrote for my embedded project (IAR EW ARM - Cortex/ARM7TDMI - Atmel SAM7, SAM4, SAMG). I am gathering data from CAN BUS (FMS), e.g. Engine Revolutions (Per Minute), and wanted to replace current fields with some class that won't break existing code (that is assigning value to it somewhere and then reading somewhere else) but would add averaging to it (to not only record last known value on demand, as it currently does, but also average since last record).



I was also thinking about prevention/solution for overflow situation, when somebody forgets to record and reset the average fast enough (goal is to gather unsigned 16bit value 100x/s and record the average every 10s), because somebody else may in the future use the system and make a mistake. So, I want it to behave reasonably - it will loose precision, but the average should still be reasonably good: it just halves the accumulator and number of samples if overflow would occur.



It turned out to be a bit more complicated than I originally anticipated, especially when I considered signed values. I am probably invoking undefined behaviour in my sum_would_overflow helper, but I know the compiler and cannot trade performance for compatibility with some exotic architecture we will never use. I can even code that little helper in assembler, but I wanted something that works and is OK C++. (I mean no log or division, these are too expensive, but bit shifts and builtins like CLZ are acceptable.)



...but this overview is not only about me and my needs, so feel free to comment/review anything without limitations. I just wanted to give you my background, nothing more.



#ifndef LIB_AVERAGER_HPP
#define LIB_AVERAGER_HPP
#include <type_traits>
//include "core/typedefs.h" .... typedef unsigned short word; typedef unsigned uint;

/// Helper to check possible overflow
/// (to reduce accumulator and counter before adding value)
template<
class Accu = uint,
class Value = word>
static inline constexpr std::enable_if_t<
std::is_unsigned<Accu>::value && std::is_unsigned<Value>::value,
bool> sum_would_overflow(Accu accu, Value value)

return accu + value < accu;

template<
class Accu = uint,
class Value = word>
static inline constexpr std::enable_if_t<
std::is_signed<Accu>::value && std::is_signed<Value>::value,
bool> sum_would_overflow(Accu accu, Value value)

auto a = static_cast<std::make_unsigned_t<Accu>>(accu);
auto v = static_cast<std::make_unsigned_t<Value>>(value);
if (accu < 0) a = -a;
if (value < 0) v = -v;
return static_cast<Accu>(a + v) < 0;


/// Current Value + Average (used in CAN Bus)
template<
class Accu = uint,
class Value = word,
class Cntr = word>
class Averager

static_assert(
std::is_signed<Value>::value == std::is_signed<Accu>::value,
"Value and Accu must both be signed or both unsigned");
static_assert(
sizeof(Accu) >= sizeof(Value),
"Accu must have at least as many bits as Value");
static_assert(
std::is_signed<Cntr>::value == false,
"Cntr must be unsigned");

Accu accu; ///< accumulator
Value curr; ///< current/last value
Cntr cntr; ///< number of samples

public:
void reset()
accu = 0;
cntr = 0;

Value value() const
return curr;

Value average() const
return cntr ? accu / cntr : curr;

void add(Value value)

operator Value() const
return curr;

Averager& operator=(Value value)
add(value);
return *this;

;
#endif


It would often be used as Averager<uint, word, word> where simple word used to be. The accumulator would be bigger, but I allowed same size in the static_assert. Some code is already using assigmnet (erpm = value) and some other fetching last value (record.addU16(erpm)). Now I want to add record.addU16(erpm.average()); erpm.reset();. And then for many other, some 16bit, some 32bit, some signed, some unsigned.



The sum_would_overflow helper was quite tricky to write and invoking the overflow to check the result, knowing 2's complement arithmetic, is probably UB, but as I said, I know the compiler and this is for embedded system, so I have to be more careful about performance than I would be when writing this for PC. I still used accu /= 2 even thoug I know accu >>= 1 would be faster for unsigned types if the compiler wasn't smart enough, but I believe IAR will optimize that... will check the assembler output and optimize my self if I find IAR is not that smart. accu /= 2 is clear, that is why I want it that way, if not hurting performance.



Accu is biggest, so I placed it first (not to invoke padding - this is 32bit ARM), Cntr would probably always be 16bit, Value can be 8/16/32, signed or unsigned.







share|improve this question





















  • The compiler is much smarter about constants then you suppose. Always write the intent, not your own tricks; the compiler knows more tricks, specific to the instruction set and even neighboring code!
    – JDługosz
    Jun 7 at 23:58
















up vote
4
down vote

favorite












This is a class/template I just wrote for my embedded project (IAR EW ARM - Cortex/ARM7TDMI - Atmel SAM7, SAM4, SAMG). I am gathering data from CAN BUS (FMS), e.g. Engine Revolutions (Per Minute), and wanted to replace current fields with some class that won't break existing code (that is assigning value to it somewhere and then reading somewhere else) but would add averaging to it (to not only record last known value on demand, as it currently does, but also average since last record).



I was also thinking about prevention/solution for overflow situation, when somebody forgets to record and reset the average fast enough (goal is to gather unsigned 16bit value 100x/s and record the average every 10s), because somebody else may in the future use the system and make a mistake. So, I want it to behave reasonably - it will loose precision, but the average should still be reasonably good: it just halves the accumulator and number of samples if overflow would occur.



It turned out to be a bit more complicated than I originally anticipated, especially when I considered signed values. I am probably invoking undefined behaviour in my sum_would_overflow helper, but I know the compiler and cannot trade performance for compatibility with some exotic architecture we will never use. I can even code that little helper in assembler, but I wanted something that works and is OK C++. (I mean no log or division, these are too expensive, but bit shifts and builtins like CLZ are acceptable.)



...but this overview is not only about me and my needs, so feel free to comment/review anything without limitations. I just wanted to give you my background, nothing more.



#ifndef LIB_AVERAGER_HPP
#define LIB_AVERAGER_HPP
#include <type_traits>
//include "core/typedefs.h" .... typedef unsigned short word; typedef unsigned uint;

/// Helper to check possible overflow
/// (to reduce accumulator and counter before adding value)
template<
class Accu = uint,
class Value = word>
static inline constexpr std::enable_if_t<
std::is_unsigned<Accu>::value && std::is_unsigned<Value>::value,
bool> sum_would_overflow(Accu accu, Value value)

return accu + value < accu;

template<
class Accu = uint,
class Value = word>
static inline constexpr std::enable_if_t<
std::is_signed<Accu>::value && std::is_signed<Value>::value,
bool> sum_would_overflow(Accu accu, Value value)

auto a = static_cast<std::make_unsigned_t<Accu>>(accu);
auto v = static_cast<std::make_unsigned_t<Value>>(value);
if (accu < 0) a = -a;
if (value < 0) v = -v;
return static_cast<Accu>(a + v) < 0;


/// Current Value + Average (used in CAN Bus)
template<
class Accu = uint,
class Value = word,
class Cntr = word>
class Averager

static_assert(
std::is_signed<Value>::value == std::is_signed<Accu>::value,
"Value and Accu must both be signed or both unsigned");
static_assert(
sizeof(Accu) >= sizeof(Value),
"Accu must have at least as many bits as Value");
static_assert(
std::is_signed<Cntr>::value == false,
"Cntr must be unsigned");

Accu accu; ///< accumulator
Value curr; ///< current/last value
Cntr cntr; ///< number of samples

public:
void reset()
accu = 0;
cntr = 0;

Value value() const
return curr;

Value average() const
return cntr ? accu / cntr : curr;

void add(Value value)

operator Value() const
return curr;

Averager& operator=(Value value)
add(value);
return *this;

;
#endif


It would often be used as Averager<uint, word, word> where simple word used to be. The accumulator would be bigger, but I allowed same size in the static_assert. Some code is already using assigmnet (erpm = value) and some other fetching last value (record.addU16(erpm)). Now I want to add record.addU16(erpm.average()); erpm.reset();. And then for many other, some 16bit, some 32bit, some signed, some unsigned.



The sum_would_overflow helper was quite tricky to write and invoking the overflow to check the result, knowing 2's complement arithmetic, is probably UB, but as I said, I know the compiler and this is for embedded system, so I have to be more careful about performance than I would be when writing this for PC. I still used accu /= 2 even thoug I know accu >>= 1 would be faster for unsigned types if the compiler wasn't smart enough, but I believe IAR will optimize that... will check the assembler output and optimize my self if I find IAR is not that smart. accu /= 2 is clear, that is why I want it that way, if not hurting performance.



Accu is biggest, so I placed it first (not to invoke padding - this is 32bit ARM), Cntr would probably always be 16bit, Value can be 8/16/32, signed or unsigned.







share|improve this question





















  • The compiler is much smarter about constants then you suppose. Always write the intent, not your own tricks; the compiler knows more tricks, specific to the instruction set and even neighboring code!
    – JDługosz
    Jun 7 at 23:58












up vote
4
down vote

favorite









up vote
4
down vote

favorite











This is a class/template I just wrote for my embedded project (IAR EW ARM - Cortex/ARM7TDMI - Atmel SAM7, SAM4, SAMG). I am gathering data from CAN BUS (FMS), e.g. Engine Revolutions (Per Minute), and wanted to replace current fields with some class that won't break existing code (that is assigning value to it somewhere and then reading somewhere else) but would add averaging to it (to not only record last known value on demand, as it currently does, but also average since last record).



I was also thinking about prevention/solution for overflow situation, when somebody forgets to record and reset the average fast enough (goal is to gather unsigned 16bit value 100x/s and record the average every 10s), because somebody else may in the future use the system and make a mistake. So, I want it to behave reasonably - it will loose precision, but the average should still be reasonably good: it just halves the accumulator and number of samples if overflow would occur.



It turned out to be a bit more complicated than I originally anticipated, especially when I considered signed values. I am probably invoking undefined behaviour in my sum_would_overflow helper, but I know the compiler and cannot trade performance for compatibility with some exotic architecture we will never use. I can even code that little helper in assembler, but I wanted something that works and is OK C++. (I mean no log or division, these are too expensive, but bit shifts and builtins like CLZ are acceptable.)



...but this overview is not only about me and my needs, so feel free to comment/review anything without limitations. I just wanted to give you my background, nothing more.



#ifndef LIB_AVERAGER_HPP
#define LIB_AVERAGER_HPP
#include <type_traits>
//include "core/typedefs.h" .... typedef unsigned short word; typedef unsigned uint;

/// Helper to check possible overflow
/// (to reduce accumulator and counter before adding value)
template<
class Accu = uint,
class Value = word>
static inline constexpr std::enable_if_t<
std::is_unsigned<Accu>::value && std::is_unsigned<Value>::value,
bool> sum_would_overflow(Accu accu, Value value)

return accu + value < accu;

template<
class Accu = uint,
class Value = word>
static inline constexpr std::enable_if_t<
std::is_signed<Accu>::value && std::is_signed<Value>::value,
bool> sum_would_overflow(Accu accu, Value value)

auto a = static_cast<std::make_unsigned_t<Accu>>(accu);
auto v = static_cast<std::make_unsigned_t<Value>>(value);
if (accu < 0) a = -a;
if (value < 0) v = -v;
return static_cast<Accu>(a + v) < 0;


/// Current Value + Average (used in CAN Bus)
template<
class Accu = uint,
class Value = word,
class Cntr = word>
class Averager

static_assert(
std::is_signed<Value>::value == std::is_signed<Accu>::value,
"Value and Accu must both be signed or both unsigned");
static_assert(
sizeof(Accu) >= sizeof(Value),
"Accu must have at least as many bits as Value");
static_assert(
std::is_signed<Cntr>::value == false,
"Cntr must be unsigned");

Accu accu; ///< accumulator
Value curr; ///< current/last value
Cntr cntr; ///< number of samples

public:
void reset()
accu = 0;
cntr = 0;

Value value() const
return curr;

Value average() const
return cntr ? accu / cntr : curr;

void add(Value value)

operator Value() const
return curr;

Averager& operator=(Value value)
add(value);
return *this;

;
#endif


It would often be used as Averager<uint, word, word> where simple word used to be. The accumulator would be bigger, but I allowed same size in the static_assert. Some code is already using assigmnet (erpm = value) and some other fetching last value (record.addU16(erpm)). Now I want to add record.addU16(erpm.average()); erpm.reset();. And then for many other, some 16bit, some 32bit, some signed, some unsigned.



The sum_would_overflow helper was quite tricky to write and invoking the overflow to check the result, knowing 2's complement arithmetic, is probably UB, but as I said, I know the compiler and this is for embedded system, so I have to be more careful about performance than I would be when writing this for PC. I still used accu /= 2 even thoug I know accu >>= 1 would be faster for unsigned types if the compiler wasn't smart enough, but I believe IAR will optimize that... will check the assembler output and optimize my self if I find IAR is not that smart. accu /= 2 is clear, that is why I want it that way, if not hurting performance.



Accu is biggest, so I placed it first (not to invoke padding - this is 32bit ARM), Cntr would probably always be 16bit, Value can be 8/16/32, signed or unsigned.







share|improve this question













This is a class/template I just wrote for my embedded project (IAR EW ARM - Cortex/ARM7TDMI - Atmel SAM7, SAM4, SAMG). I am gathering data from CAN BUS (FMS), e.g. Engine Revolutions (Per Minute), and wanted to replace current fields with some class that won't break existing code (that is assigning value to it somewhere and then reading somewhere else) but would add averaging to it (to not only record last known value on demand, as it currently does, but also average since last record).



I was also thinking about prevention/solution for overflow situation, when somebody forgets to record and reset the average fast enough (goal is to gather unsigned 16bit value 100x/s and record the average every 10s), because somebody else may in the future use the system and make a mistake. So, I want it to behave reasonably - it will loose precision, but the average should still be reasonably good: it just halves the accumulator and number of samples if overflow would occur.



It turned out to be a bit more complicated than I originally anticipated, especially when I considered signed values. I am probably invoking undefined behaviour in my sum_would_overflow helper, but I know the compiler and cannot trade performance for compatibility with some exotic architecture we will never use. I can even code that little helper in assembler, but I wanted something that works and is OK C++. (I mean no log or division, these are too expensive, but bit shifts and builtins like CLZ are acceptable.)



...but this overview is not only about me and my needs, so feel free to comment/review anything without limitations. I just wanted to give you my background, nothing more.



#ifndef LIB_AVERAGER_HPP
#define LIB_AVERAGER_HPP
#include <type_traits>
//include "core/typedefs.h" .... typedef unsigned short word; typedef unsigned uint;

/// Helper to check possible overflow
/// (to reduce accumulator and counter before adding value)
template<
class Accu = uint,
class Value = word>
static inline constexpr std::enable_if_t<
std::is_unsigned<Accu>::value && std::is_unsigned<Value>::value,
bool> sum_would_overflow(Accu accu, Value value)

return accu + value < accu;

template<
class Accu = uint,
class Value = word>
static inline constexpr std::enable_if_t<
std::is_signed<Accu>::value && std::is_signed<Value>::value,
bool> sum_would_overflow(Accu accu, Value value)

auto a = static_cast<std::make_unsigned_t<Accu>>(accu);
auto v = static_cast<std::make_unsigned_t<Value>>(value);
if (accu < 0) a = -a;
if (value < 0) v = -v;
return static_cast<Accu>(a + v) < 0;


/// Current Value + Average (used in CAN Bus)
template<
class Accu = uint,
class Value = word,
class Cntr = word>
class Averager

static_assert(
std::is_signed<Value>::value == std::is_signed<Accu>::value,
"Value and Accu must both be signed or both unsigned");
static_assert(
sizeof(Accu) >= sizeof(Value),
"Accu must have at least as many bits as Value");
static_assert(
std::is_signed<Cntr>::value == false,
"Cntr must be unsigned");

Accu accu; ///< accumulator
Value curr; ///< current/last value
Cntr cntr; ///< number of samples

public:
void reset()
accu = 0;
cntr = 0;

Value value() const
return curr;

Value average() const
return cntr ? accu / cntr : curr;

void add(Value value)

operator Value() const
return curr;

Averager& operator=(Value value)
add(value);
return *this;

;
#endif


It would often be used as Averager<uint, word, word> where simple word used to be. The accumulator would be bigger, but I allowed same size in the static_assert. Some code is already using assigmnet (erpm = value) and some other fetching last value (record.addU16(erpm)). Now I want to add record.addU16(erpm.average()); erpm.reset();. And then for many other, some 16bit, some 32bit, some signed, some unsigned.



The sum_would_overflow helper was quite tricky to write and invoking the overflow to check the result, knowing 2's complement arithmetic, is probably UB, but as I said, I know the compiler and this is for embedded system, so I have to be more careful about performance than I would be when writing this for PC. I still used accu /= 2 even thoug I know accu >>= 1 would be faster for unsigned types if the compiler wasn't smart enough, but I believe IAR will optimize that... will check the assembler output and optimize my self if I find IAR is not that smart. accu /= 2 is clear, that is why I want it that way, if not hurting performance.



Accu is biggest, so I placed it first (not to invoke padding - this is 32bit ARM), Cntr would probably always be 16bit, Value can be 8/16/32, signed or unsigned.









share|improve this question












share|improve this question




share|improve this question








edited Jun 7 at 14:24









200_success

123k14143399




123k14143399









asked Jun 7 at 14:10









firda

2,184525




2,184525











  • The compiler is much smarter about constants then you suppose. Always write the intent, not your own tricks; the compiler knows more tricks, specific to the instruction set and even neighboring code!
    – JDługosz
    Jun 7 at 23:58
















  • The compiler is much smarter about constants then you suppose. Always write the intent, not your own tricks; the compiler knows more tricks, specific to the instruction set and even neighboring code!
    – JDługosz
    Jun 7 at 23:58















The compiler is much smarter about constants then you suppose. Always write the intent, not your own tricks; the compiler knows more tricks, specific to the instruction set and even neighboring code!
– JDługosz
Jun 7 at 23:58




The compiler is much smarter about constants then you suppose. Always write the intent, not your own tricks; the compiler knows more tricks, specific to the instruction set and even neighboring code!
– JDługosz
Jun 7 at 23:58















active

oldest

votes











Your Answer




StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");

StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: false,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);



);








 

draft saved


draft discarded


















StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f196040%2faveraging-accumulator-template-with-overflow-detection-prevention%23new-answer', 'question_page');

);

Post as a guest



































active

oldest

votes













active

oldest

votes









active

oldest

votes






active

oldest

votes










 

draft saved


draft discarded


























 


draft saved


draft discarded














StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f196040%2faveraging-accumulator-template-with-overflow-detection-prevention%23new-answer', 'question_page');

);

Post as a guest













































































Popular posts from this blog

Chat program with C++ and SFML

Function to Return a JSON Like Objects Using VBA Collections and Arrays

Will my employers contract hold up in court?