Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: addition of potentialSet class #1991
base: develop
Are you sure you want to change the base?
feat: addition of potentialSet class #1991
Changes from 17 commits
1ad2d1b
bae42c4
2e107e9
2be06a0
57a0c5a
ca5bb3e
6f84266
88e0bbb
8e345d2
b2d20d7
677a2a2
dd10495
b1bd16b
87e8029
b5467b8
cc37441
16e8b19
c4f2056
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You definitely don't want to do this division every time you add a set of potentials, just once at the end!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 addpotentials
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 doingpop_back
rather thanpop_front
. TheAveraging
class would sort all this out for you, but I have suggested the necessary modifications to clean this up anyway.