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 bugs in espressomd.analyze module #4534

Merged
merged 3 commits into from
Jul 18, 2022

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Jul 12, 2022

Description of changes:

  • fix particle double-counting bug in the structure factor analysis code
  • fix distance calculation bug in the particle distribution analysis code when r_max > np.sum(system.box_l)
  • remove unused v_kappa analysis code

This feature is unused, untested and undocumented.
The analysis.structure_factor() method could count particles twice
when the corresponding particle type appeared twice in the sf_types
argument.
Use r_max + 1 as the initial minimal distance instead of an
arbitrary value when calculating the RDF of the minimal distance
with r_max larger than the box size.
@jngrad jngrad added this to the Espresso 4.3.0 milestone Jul 12, 2022
@jngrad jngrad requested a review from reinaual July 12, 2022 08:06
@jngrad jngrad marked this pull request as ready for review July 12, 2022 08:55
@jngrad jngrad added the automerge Merge with kodiak label Jul 18, 2022
@kodiakhq kodiakhq bot merged commit 50cde0d into espressomd:python Jul 18, 2022
@jngrad jngrad deleted the analysis_bugfix branch July 18, 2022 13:55
@jngrad jngrad modified the milestones: Espresso 4.3.0, Espresso 4.2.1 Nov 21, 2022
jngrad pushed a commit to jngrad/espresso that referenced this pull request Nov 28, 2022
Description of changes:
- fix particle double-counting bug in the structure factor analysis code
- fix distance calculation bug in the particle distribution analysis code when `r_max > np.sum(system.box_l)`
- remove unused v_kappa analysis code
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.

2 participants