feat: addition of potentialSet class #1991
Conversation
Danbr4d
changed the title
Co-authored-by: Adam Washington <adam.washington@stfc.ac.uk>
Co-authored-by: Adam Washington <adam.washington@stfc.ac.uk>
| std::map<std::string, EPData> averagedPotentials = potentials; | ||
|
|
||
| averagedPotentialsStore.emplace_back(potentials); | ||
| // Check if ran the right amount of iterations before averaging | ||
| if (averagedPotentialsStore.size() > averagingLength_) | ||
| { | ||
| averagedPotentialsStore.pop_back(); | ||
| } | ||
|
|
||
| // Average the potentials and replace the map with the new averaged | ||
| for (const auto &pots : averagedPotentialsStore) | ||
| { | ||
| for (auto &&[key, epData] : pots) | ||
| { | ||
| averagedPotentials[key].ep += epData.ep; | ||
| averagedPotentials[key].ep /= averagingLength_.value(); | ||
| } | ||
| } | ||
| potentials = averagedPotentials; | ||
|
|
There was a problem hiding this comment.
Could this section be the issue? The logic here doesn't quite add up, since you store the current potentials as the beginning of averagePotentials, then add potentials to the store, then immediately remove it if you have exceeded your averaging length (which past a certain point you always will) and then do the sum over all the store potentials. That will give you N+1 sets of potentials contributing to the average, rather than the N you want, and which will always contain the first N potential sets you ever generate at the beginning of the simulation (since you're doing pop_back rather than pop_front. The Averaging class would sort all this out for you, but I have suggested the necessary modifications to clean this up anyway.
| @@ -53,6 +53,26 @@ Module::ExecutionResult EPSRManagerModule::process(ModuleContext &moduleContext) | |||
| for (auto &&[key, epData] : potentials) | |||
| epData.ep /= epData.count; | |||
|
|
|||
| std::map<std::string, EPData> averagedPotentials = potentials; | |||
|
|
|||
| averagedPotentialsStore.emplace_back(potentials); | |||
There was a problem hiding this comment.
| averagedPotentialsStore.emplace_back(potentials); |
| // Check if ran the right amount of iterations before averaging | ||
| if (averagedPotentialsStore.size() > averagingLength_) | ||
| { | ||
| averagedPotentialsStore.pop_back(); | ||
| } |
There was a problem hiding this comment.
| // Check if ran the right amount of iterations before averaging | |
| if (averagedPotentialsStore.size() > averagingLength_) | |
| { | |
| averagedPotentialsStore.pop_back(); | |
| } | |
| // Limit data to average to the most recent N-1 sets | |
| while (averagedPotentialsStore.size() >= averagingLength_) | |
| { | |
| averagedPotentialsStore.pop_front(); | |
| } |
| averagedPotentials[key].ep /= averagingLength_.value(); | ||
| } | ||
| } | ||
| potentials = averagedPotentials; |
There was a problem hiding this comment.
| potentials = averagedPotentials; | |
| // Add the current potentials (before averaging) to the average store before we overwrite them | |
| averagePotentialsStore.emplace_back(potentials); | |
| // Set new averaged potentials | |
| potentials = averagedPotentials; |
| for (auto &&[key, epData] : pots) | ||
| { | ||
| averagedPotentials[key].ep += epData.ep; | ||
| averagedPotentials[key].ep /= averagingLength_.value(); |
There was a problem hiding this comment.
You definitely don't want to do this division every time you add a set of potentials, just once at the end!
Addition of potentialSet class for easier averaging of potentials.