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.

Sign up for GitHub

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

Jump to bottom

Fix polymer generation #3484

Merged
merged 6 commits into from
Feb 14, 2020
Merged

Fix polymer generation #3484

merged 6 commits into from
Feb 14, 2020

Conversation

fweik
Copy link
Contributor

@fweik fweik commented Feb 14, 2020

Fixes #3483.

Description of changes:

  • Obey minimum image convention
  • To not falsely pre-populate all positions with the zero vector
  • Refactored generation loop to make it more redable
  • Check for constraint violations between starting positions

@fweik fweik requested a review from jngrad February 14, 2020 15:15
@codecov
Copy link

codecov bot commented Feb 14, 2020 •
edited
Loading

Codecov Report

Merging #3484 into python will increase coverage by <1%.
The diff coverage is 92%.

Impacted file tree graph

@@           Coverage Diff           @@
##           python   #3484    +/-   ##
=======================================
+ Coverage      87%     87%   +<1%     
=======================================
  Files         537     537            
  Lines       24460   24449    -11     
=======================================
- Hits        21291   21284     -7     
+ Misses       3169    3165     -4
Impacted Files Coverage Δ
src/core/polymer.cpp 95% <92%> (+5%) ⬆️
src/core/particle_data.cpp 97% <0%> (-1%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb56b21...e98e913. Read the comment docs.

jngrad
jngrad requested changes Feb 14, 2020
View reviewed changes
src/core/polymer.cpp Outdated Show resolved Hide resolved
src/core/polymer.cpp Outdated Show resolved Hide resolved
@jngrad
Copy link
Member

jngrad commented Feb 14, 2020

This is a lot better! The polymer_linear.py test doesn't fail anymore, even with a different seed at each execution. This isn't the first time that a bug is ignored by setting a magic seed in the test script...

@jngrad jngrad added this to the Espresso 4.1.3 milestone Feb 14, 2020
@fweik @jngrad
Fixed typos
e98e913 
Co-Authored-By: Jean-Noël Grad <jgrad@icp.uni-stuttgart.de>
@jngrad jngrad added the automerge Merge with kodiak label Feb 14, 2020
@kodiakhq kodiakhq bot merged commit 9c2c70d into espressomd:python Feb 14, 2020
@fweik fweik deleted the polymer branch February 14, 2020 20:26
@jngrad jngrad mentioned this pull request Feb 17, 2020
Merged
kodiakhq bot added a commit that referenced this pull request Feb 17, 2020
@kodiakhq
Remove infinite loop in linear polymer (#3491)
f9ae0e8 
Description of changes:
- fix infinite loop in `draw_polymer_positions()` (regression from #3484)
- remove fixed seed in the `polymer_linear.py` test that used to hide a bug in the previous implementation of `draw_polymer_positions()`
@KaiSzuttor KaiSzuttor mentioned this pull request Feb 18, 2020
Closed
jngrad added a commit to jngrad/espresso that referenced this pull request Feb 19, 2020
@jngrad
Fix electrophoresis sample test
c68c018 
The two values changed after the polymer refactoring in espressomd#3484.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jngrad jngrad jngrad approved these changes

Assignees
No one assigned
Labels
automerge Merge with kodiak BugFix Core
Projects
None yet
Milestone
Espresso 4.1.3
Development

Successfully merging this pull request may close these issues.

Polymer generation does not obey minimum image convention
2 participants
@fweik @jngrad