Skip to content
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

Fix rcb balance issue #928

Merged
merged 3 commits into from Aug 9, 2018
Merged

Conversation

akohlmey
Copy link
Member

Purpose

implement changes suggested in issue #888

@akohlmey akohlmey self-assigned this May 23, 2018
correct issue reported in comment at lammps#911
In src/rcb.cpp:460 there is an if (smaller > largest).
now if we have one particle you will see that lo[] = hi[] and because
of this smaller == largest == 0 for all values of dim. This causes
this particular part of the code to never be run. In particular the
memcpy inside this if is never executed. This causes an unitialized
memory access in line 472. Additionally, dim is initialized with -1
and thus the accesses in 484 and 485 are problematic. Additionally,
valuehalf_select is never initialized either.

closes lammps#888
@akohlmey akohlmey assigned sjplimp and unassigned akohlmey May 23, 2018
@akohlmey
Copy link
Member Author

@sjplimp here is the "lost commit" rebased to the current master and another doc update commit, that may have been lost in the process as well. FYI, the branch is "open", so you can pull it and push changes to it.

@sjplimp
Copy link
Contributor

sjplimp commented Aug 9, 2018

changed the corner case with shrink-wrap size = 0.0 to reset it to original box
size, not something based on neighbor skin, which could produce a box size
larger than original box

@sjplimp sjplimp assigned akohlmey and unassigned sjplimp Aug 9, 2018
@akohlmey akohlmey assigned sjplimp and unassigned akohlmey Aug 9, 2018
@akohlmey
Copy link
Member Author

akohlmey commented Aug 9, 2018

@sjplimp it is a bit difficult to see what are the real final changes that are going to be applied to master with a pull request that was created such a long time ago. however, i did a manual test merge in my local repository and compared that to master and it looks ok there. so i am handing this back to you for merging.

@sjplimp sjplimp merged commit 6663fbe into lammps:master Aug 9, 2018
@sjplimp
Copy link
Contributor

sjplimp commented Aug 9, 2018

@akohlmey I made my changes to all 3 files based on the current master files, so it should be fine.

@akohlmey akohlmey deleted the fix-rcb-balance-issue branch August 10, 2018 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants