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

lj-demo.py is broken #4347

Closed
jngrad opened this issue Sep 14, 2021 · 3 comments · Fixed by #4482
Closed

lj-demo.py is broken #4347

jngrad opened this issue Sep 14, 2021 · 3 comments · Fixed by #4482

Comments

@jngrad
Copy link
Member

jngrad commented Sep 14, 2021

The physics in samples/lj-demo.py is broken since ESPResSo 4.0. The script is also completely untested, and has diverged, e.g. the NpT option doesn't work because the integrator function signature changed in 4.1. It's possible to fix the sample without a MIDI board.

Tasks:

  • Remove the phase diagram plot. It is unclear how it was obtained, and the dot temporarily moves into the solid phase when the volume is increased, which is not correct; as the box volume increases, it takes time for the fluid to occupy the newly available space, which is why the dot temporarily enters the solid region. However a phase diagram describes the state of a system at equilibrium, and during the time it takes for the fluid to occupy the new space, the system is strongly out-of-equilibrium. I also couldn't find an analytical solution for the phase diagram of a simple LJ system, and according to the literature it depends strongly on the cutoff value for the LJ potential (Smit 1992, doi:10.1063/1.462271).
  • Fix the overflow issue when the GUI sliders are moved too quickly.
  • Fix NpT error caused by the change of signature of the NpT integrator.
  • Add a minimal test case, if feasible. It's not going to be pretty. testsuite/scripts/samples/test_visualization_npt.py can be used as template, should probably work with the following patch:
patch (click to unroll)
def disable_GUI_and_MIDI(code):
    # disable MIDI and traits interface
    breakpoint = 'from traits.api import HasTraits, Any, Range, List, Enum, Float\nfrom traitsui.api import View, Group, Item, CheckListEditor, RangeEditor'
    assert breakpoint in code
    code.replace(
        breakpoint,
        'from unittest.mock import MagicMock()\n' +
        '\n'.join(
            f'{x} = MagicMock()' for y in breakpoint.split('\n') for x in y.split(' import ')[1].split(', ')))
    breakpoint = 'system.thermostat.set_langevin(kT=controls.temperature, gamma=1.0)'
    assert breakpoint in code
    code.replace(breakpoint, 'system.thermostat.set_langevin(kT=1, gamma=1.0)')
    breakpoint = 'new_box = np.ones(3) * controls.volume**(1. / 3.)'
    assert breakpoint in code
    code.replace(breakpoint, 'new_box = [3, 3, 3]')
    breakpoint = 'new_part = controls.number_of_particles'
    assert breakpoint in code
    code.replace(breakpoint, 'new_part = 100')
    # integrate without visualizer
    breakpoint = 't = Thread(target=main_thread)'
    assert breakpoint in code
    code = code.split(breakpoint)[0] + 'controls = Controls()\nmain_thread()'
    return code


sample, skipIfMissingFeatures = importlib_wrapper.configure_and_import(
    "@SAMPLES_DIR@/lj-demo.py", int_n_times=10,
    substitutions=disable_GUI_and_MIDI, cmd_arguments=["--opengl"])
@davidbbeyer
Copy link
Contributor

I will take a look.

@jngrad
Copy link
Member Author

jngrad commented Sep 28, 2021

@davidbbeyer any update?

@davidbbeyer
Copy link
Contributor

Sorry, I haven't fixed the overflow issue yet, I will do it in the next days.

kodiakhq bot added a commit that referenced this issue Mar 22, 2022
Description of changes:
- improve exception handling in MPI code (partial fix for #4219)
   - several runtime errors used to be queued for far too long and would only surface during integration
      - runtime errors from non-bonded interactions instantiated with a cutoff too large now immediately raise an exception, from which the user needs to recover by either reducing the cutoff or disabling the interaction
      - runtime errors from virtual sites relative applied to particles too far away now immediately raise an exception, from which the user needs to recover by increasing the minimum global cutoff
   - all automatically-generated script interface methods used to call `handle_errors("")` with an empty string; now it's called with the method name to help identify which feature threw the exception
   - virtual sites fatal errors are now runtime errors
   - quaternion fatal errors are now runtime errors
   - invalid particle ids fatal errors are now value errors
- fix bugs
   - virtual sites can no longer relate to themselves
   - particles can no longer be created with a negative id
   - LB node property `boundary` no longer returns a random integer when `LB_BOUNDARIES` is not compiled in
- improve code coverage
   - common exceptions are now properly covered by tests (e.g. error messages from the cell system and integrator)
   - rotation code is now unit tested
   - h5md exceptions are now tested
- fix regressions in the testsuite
   - checkpoint tests were only running on 1 MPI core
   - the LB VTK had a wall boundary misplaced
   - virtual sites relative test cases can now run in any order
- fix regression in the OpenGL visualizer
   - activating the `LB_draw_boundaries` option without any other `LB_draw_*` option no longer throws an exception
   - rescaling the box size in `lj-demo.py` now properly rescales the particle coordinates (partial fix for #4347)
@kodiakhq kodiakhq bot closed this as completed in #4482 Apr 4, 2022
kodiakhq bot added a commit that referenced this issue Apr 4, 2022
Description of changes:
- API change: rename `espressomd.reaction_ensemble` to `espressomd.reaction_methods`
- feature configuration: remove obsolete `EXPERIMENTAL_FEATURES`
- script interface:
   - use shared pointers instead of `new` expressions in the script interface
   - make type conversion error messages less ambiguous by putting quotes around C++ types
   - require EK node grid indices to be exactly 3 integers (2 indices would skip the exception and return `None`)
- python interface:
   - replace `from X import Y` by `import X` and `X.Y` in samples and python interface
   - use `tqdm`, `numpy` and f-strings more often in the samples
   - remove `lj-demo.py` (fixes  #4347)
   - remove mentions to non-existent functions in the user guide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants