Skip to content

feat: Data history storage (for averaging) - #2206

Merged
trisyoungs merged 4 commits into
develop2from
dissolve2/averaging
Jul 28, 2025
Merged

feat: Data history storage (for averaging)#2206
trisyoungs merged 4 commits into
develop2from
dissolve2/averaging

Conversation

@trisyoungs

Copy link
Copy Markdown
Member

Follows #2204 and must be merged before #2203.

This PR introduces a new History template class to generalise (and simplify) the functionality of the old Averaging namespace. The idea is straightforward - introduce a sort of managed std::vector of data which can be easily formed into an average as well as serialised if required (e.g. as node data will require).

Comment thread src/math/history.h
Comment on lines +24 to +32
// Prune old data to get to the averagingLength
while (history_.size() > averagingLength)
history_.erase(history_.begin());

// Perform averaging of the datasets that we have
T averaged;
auto weight = 1.0 / history_.size();
for (auto &data : history_)
averaged += *data * weight;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Prune old data to get to the averagingLength
while (history_.size() > averagingLength)
history_.erase(history_.begin());
// Perform averaging of the datasets that we have
T averaged;
auto weight = 1.0 / history_.size();
for (auto &data : history_)
averaged += *data * weight;
// How many items to average
auto length = history.size() < averagingLength ? history.size() : averagingLength;
// Perform averaging of the datasets that we have
T averaged;
auto weight = 1.0 / length;
for (auto &data : std::span(history_.rbegin(), ristory_.rbegin()+length))
averaged += *data * weight;

This is an alternate implementation that doesn't lose history. This would allow us to look at multiple averaging lengths simultaneously (the current implementation essentially locks you into the shortest length). The disadvantage is that it can grow without bound, though that could be fixed by replacing the vector with a ring buffer.

This isn't a necessary change - just a suggestion.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting thought. While I like the idea, my concern is that this could add a lot of bloat as many of the objects being stored here are pretty chunky (I'm looking at you, PartialSet). As such, I would prefer to stick to the ultra-simple, shortest-length version for the time being.

@rprospero rprospero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Also gives us a nice framework for other stats (e.g. standard deviation, median, etc.)

One small suggested change, but, otherwise, things look good.

Base automatically changed from dissolve2/single-configuration-gr to develop2 July 25, 2025 12:40
@trisyoungs
trisyoungs force-pushed the dissolve2/averaging branch from 82378c6 to ae91a7b Compare July 25, 2025 15:30
@trisyoungs
trisyoungs marked this pull request as ready for review July 28, 2025 14:07
@trisyoungs
trisyoungs merged commit dceb4c0 into develop2 Jul 28, 2025
11 checks passed
@trisyoungs
trisyoungs deleted the dissolve2/averaging branch July 28, 2025 14:09
rprospero pushed a commit that referenced this pull request Sep 5, 2025
Co-authored-by: Tristan Youngs <trisyoungs@googlemail.com>
rprospero pushed a commit that referenced this pull request Feb 4, 2026
Co-authored-by: Tristan Youngs <trisyoungs@googlemail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants