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 errors in LB+Lees-Edwards #4829

Closed
wants to merge 18 commits into from

Conversation

RudolfWeeber
Copy link
Contributor

contains #4827

This

  • fixs index calculation in the LB Walberla sweep doing the interpolation of populations at the Lees-Edwards boundary
  • adds missing Lees-Edwards velocity shift in the lb particle coupling, when a couplign is poerformed at a particle positoin +- box_length (for ghosts)
  • adds more testing of LE+LB particle coupling
  • LB+Lees Edwards fixes
  • Allow Langevin +LB
  • LB: Fix rounding error in particle coupling
  • LB+particles: guard against missed coupling due to round-off error
  • Formatting

Fixes #

Description of changes:

Copy link
Contributor

@christophlohrmann christophlohrmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good but I'm no expert in lees edwards

src/core/lb/particle_coupling.cpp Outdated Show resolved Hide resolved
if (normal_shift < -std::numeric_limits<double>::epsilon()) {
vel_shift[le.shear_direction] += le.shear_velocity;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it on purpose that nothing happens if the normal shift is inbetween -epsilon and epsilon?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. normal_shift contains multiples of box_l or 0. We basally just want +-1 or 0, but tollerant to rounding errors. However, in one of teh two instances, where this appears, that integer is directly available in the shift variable, so I used that instead. In the other instance, I tried to make it clear by renaming the variables.


int target_idx = cell[dir];
auto ghost_fold = [dim](FloatType x) {
while (x > dim) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this also looks a lot like a modulo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't, actually. A modulo would be x>=n or x<n. Here, it's x>n or x<-1. This is, ebcause Walberla uses -1 and n to address the ghost layers.

# across the Lees Edwads boundary conditoin
# if offset ^1<.5, it couples to the left neighbor
# otherwise to the right
if (offset % 1) >= .5: extra = 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ternary operator would look nicer than if-else without indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced by round()

testsuite/python/lb_lees_edwards_particle_coupling.py Outdated Show resolved Hide resolved
testsuite/python/lb_lees_edwards_particle_coupling.py Outdated Show resolved Hide resolved
testsuite/python/lb_lees_edwards_particle_coupling.py Outdated Show resolved Hide resolved
testsuite/python/lb_lees_edwards_particle_coupling.py Outdated Show resolved Hide resolved
testsuite/python/lb_lees_edwards_particle_coupling.py Outdated Show resolved Hide resolved
@espresso-ci
Copy link

Your pull request does not meet our code formatting rules. To fix this, please do one of the following:

  • You can download a patch with my suggested changes here, inspect it and make changes manually.
  • You can directly apply it to your repository by running curl https://gitlab.icp.uni-stuttgart.de/espressomd/espresso/-/jobs/361104/artifacts/raw/style.patch | git apply -.
  • You can run maintainer/CI/fix_style.sh to automatically fix your coding style. This is the same command that I have executed to generate the patch above, but it requires certain tools to be installed on your computer.

You can run gitlab-runner exec docker style afterwards to check if your changes worked out properly.

Please note that there are often multiple ways to correctly format code. As I am just a robot, I sometimes fail to identify the most aesthetically pleasing way. So please look over my suggested changes and adapt them where the style does not make sense.

@espresso-ci
Copy link

Your pull request does not meet our code style rules. Pylint summary:

************* Module lb_lees_edwards_particle_coupling
testsuite/python/lb_lees_edwards_particle_coupling.py:110:4: W0612: Unused variable 'indices_shifted_left' (unused-variable)
testsuite/python/lb_lees_edwards_particle_coupling.py:111:4: W0612: Unused variable 'indices_shifted_right' (unused-variable)
testsuite/python/lb_lees_edwards_particle_coupling.py:329:8: W0612: Unused variable 'v_shear' (unused-variable)
testsuite/python/lb_lees_edwards_particle_coupling.py:344:12: W0612: Unused variable 'i' (unused-variable)

You can generate these warnings with maintainer/CI/fix_style.sh. This is the same command that I have executed to generate the log above.

@RudolfWeeber
Copy link
Contributor Author

I'm closing this in favor of a new pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants