Skip to content

Commit

Permalink
Fix checkpointing bug and improve CI stability (#4986)
Browse files Browse the repository at this point in the history
Fixes #4964

Description of changes:
- bugfix: checkpointing can be used more than once again
   - the regression was introduced by the ASE bindings
   - the checkpointing tests now save the checkpoint a second time
- add missing include guards in the testsuite
- fix Doxygen CI job
  • Loading branch information
kodiakhq[bot] authored Aug 23, 2024
2 parents 650d45e + 940373f commit e3ac3af
Show file tree
Hide file tree
Showing 14 changed files with 107 additions and 48 deletions.
2 changes: 2 additions & 0 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ cuda12-coverage:
with_stokesian_dynamics: 'true'
script:
- bash maintainer/CI/build_cmake.sh
timeout: 90m
tags:
- espresso
- cuda
Expand Down Expand Up @@ -252,6 +253,7 @@ cuda12-maxset:
paths:
- build/
expire_in: 1 week
timeout: 90m
tags:
- espresso
- cuda
Expand Down
14 changes: 6 additions & 8 deletions doc/doxygen/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (C) 2015-2022 The ESPResSo project
# Copyright (C) 2015-2024 The ESPResSo project
#
# This file is part of ESPResSo.
#
Expand Down Expand Up @@ -28,25 +28,23 @@ if(DOXYGEN_FOUND)
set(DOXYGEN_BIB_IN ${CMAKE_SOURCE_DIR}/doc/bibliography.bib)
set(DOXYGEN_BIB_OUT ${CMAKE_CURRENT_BINARY_DIR}/bibliography.bib)

# transform BibTeX DOI fields into URL fields (bibliographic styles available
# to Doxygen do not process the DOI field)
add_custom_command(
OUTPUT ${DOXYGEN_BIB_OUT}
COMMAND
sed -r "'s_^ *doi *= *([^0-9]+)(10\\.[0-9]+)_url=\\1https://doi.org/\\2_'"
${DOXYGEN_BIB_IN} > ${DOXYGEN_BIB_OUT}
${CMAKE_COMMAND} -DINPUT=${DOXYGEN_BIB_IN} -DOUTPUT=${DOXYGEN_BIB_OUT} -P
${CMAKE_CURRENT_SOURCE_DIR}/bibtex_preprocess.cmake
DEPENDS ${DOXYGEN_BIB_IN})

set(DOXYFILE_IN ${CMAKE_CURRENT_SOURCE_DIR}/Doxyfile.in)
set(DOXYFILE_OUT ${CMAKE_CURRENT_BINARY_DIR}/Doxyfile)
set(CITELIST ${CMAKE_CURRENT_BINARY_DIR}/html/citelist.html)
set(DOXYGEN_CITELIST ${CMAKE_CURRENT_BINARY_DIR}/html/citelist.html)

configure_file(${DOXYFILE_IN} ${DOXYFILE_OUT} @ONLY)

add_custom_target(
doxygen COMMAND ${DOXYGEN_EXECUTABLE} ${DOXYFILE_OUT}
COMMAND sed -ri "s/ textsuperscript (\\S+) /<sup>\\1<\\/sup> /" ${CITELIST}
COMMAND sed -ri "s/ textsubscript (\\S+) /<sub>\\1<\\/sub> /" ${CITELIST}
COMMAND ${CMAKE_COMMAND} -DBIBLIOGRAPHY=${DOXYGEN_CITELIST} -P
${CMAKE_CURRENT_SOURCE_DIR}/bibtex_postprocess.cmake
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
COMMENT "Generating API documentation with Doxygen"
DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/doxy-features ${DOXYGEN_BIB_OUT}
Expand Down
26 changes: 26 additions & 0 deletions doc/doxygen/bibtex_postprocess.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#
# Copyright (C) 2023-2024 The ESPResSo project
#
# This file is part of ESPResSo.
#
# ESPResSo is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# ESPResSo is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#

if(EXISTS "${BIBLIOGRAPHY}")
file(READ "${BIBLIOGRAPHY}" FILE_CONTENTS)
# convert unsupported LaTeX formatting commands to HTML
string(REGEX REPLACE " textsuperscript ([^ ]+) " "<sup>\\1</sup> " FILE_CONTENTS "${FILE_CONTENTS}")
string(REGEX REPLACE " textsubscript ([^ ]+) " "<sub>\\1</sub> " FILE_CONTENTS "${FILE_CONTENTS}")
file(WRITE "${BIBLIOGRAPHY}" "${FILE_CONTENTS}")
endif()
29 changes: 29 additions & 0 deletions doc/doxygen/bibtex_preprocess.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#
# Copyright (C) 2019-2024 The ESPResSo project
#
# This file is part of ESPResSo.
#
# ESPResSo is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# ESPResSo is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#

file(READ "${INPUT}" FILE_CONTENTS)

# transform BibTeX DOI fields into URL fields (bibliographic styles available
# to Doxygen do not process the DOI field)
string(REGEX REPLACE
"([\r\n])[\t ]*doi *= *([\\{\"]+)(10\\.[0-9]+)"
"\\1url=\\2https://doi.org/\\3"
FILE_CONTENTS "${FILE_CONTENTS}")

file(WRITE "${OUTPUT}" "${FILE_CONTENTS}")
1 change: 1 addition & 0 deletions src/core/cell_system/CellStructure.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ struct CellStructure : public System::Leaf<CellStructure> {
* @brief Set the particle decomposition to @ref RegularDecomposition.
*
* @param range Interaction range.
* @param fully_connected_boundary neighbor cell directions for Lees-Edwards.
*/
void set_regular_decomposition(
double range,
Expand Down
2 changes: 1 addition & 1 deletion src/python/espressomd/checkpointing.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ def get_last_checkpoint_index(self):
def save(self, checkpoint_index=None):
"""
Saves all registered python objects in the given checkpoint directory
using cPickle.
using pickle.
"""
# get attributes of registered objects
Expand Down
23 changes: 12 additions & 11 deletions src/python/espressomd/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,18 +135,19 @@ def __init__(self, **kwargs):
if "sip" in kwargs:
super().__init__(**kwargs)
self._setup_atexit()
return
super().__init__(_regular_constructor=True, **kwargs)
if has_features("CUDA"):
self.cuda_init_handle = cuda_init.CudaInitHandle()
if has_features("WALBERLA"):
self._lb = None
self._ekcontainer = None
self._ase_interface = None
else:
super().__init__(_regular_constructor=True, **kwargs)
if has_features("CUDA"):
self.cuda_init_handle = cuda_init.CudaInitHandle()
if has_features("WALBERLA"):
self._lb = None
self._ekcontainer = None

# lock class
self.call_method("lock_system_creation")
self._setup_atexit()
# lock class
self.call_method("lock_system_creation")
self._setup_atexit()

self._ase_interface = None

def _setup_atexit(self):
import atexit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,11 @@ foreach(precision double_precision single_precision)
FrictionCouplingKernel_${precision}.cpp
FixedFlux_${precision}.cpp
Dirichlet_${precision}.cpp)
# disable UBSAN on large files due to excessive compile times
set_source_files_properties(
AdvectiveFluxKernel_${precision}.cpp
DiffusiveFluxKernelWithElectrostatic_${precision}.cpp
DiffusiveFluxKernelWithElectrostaticThermalized_${precision}.cpp
TARGET_DIRECTORY espresso_walberla PROPERTIES COMPILE_FLAGS
-fno-sanitize=undefined)
endforeach()
5 changes: 3 additions & 2 deletions testsuite/python/hybrid_decomposition.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ def prepare_hybrid_setup(self, n_part_small=0, n_part_large=0):
initial velocities; cutoff of small particles is 2.5.
"""
assert espressomd.has_features(["LENNARD_JONES"])
box_l = self.system.box_l[0]
if n_part_small > 0:
self.system.part.add(
Expand Down Expand Up @@ -249,7 +250,7 @@ def add_particles(self, charge=False, dip=False):

@ut.skipIf(system.cell_system.get_state()["n_nodes"] != 1,
"Skipping test: only runs for n_nodes == 1")
@utx.skipIfMissingFeatures(["P3M"])
@utx.skipIfMissingFeatures(["P3M", "LENNARD_JONES"])
def test_against_regular_p3m(self):
import espressomd.electrostatics

Expand All @@ -264,7 +265,7 @@ def test_against_regular_p3m(self):

@ut.skipIf(system.cell_system.get_state()["n_nodes"] != 1,
"Skipping test: only runs for n_nodes == 1")
@utx.skipIfMissingFeatures(["DP3M"])
@utx.skipIfMissingFeatures(["DP3M", "LENNARD_JONES"])
def test_against_regular_dp3m(self):
import espressomd.magnetostatics

Expand Down
1 change: 1 addition & 0 deletions testsuite/python/lb.py
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,7 @@ def test_viscous_coupling_rounding(self):
self.system.integrator.run(1)
self.assertTrue(np.all(p.f != 0.0))

@utx.skipIfMissingFeatures("VIRTUAL_SITES_INERTIALESS_TRACERS")
def test_tracers_coupling_rounding(self):
import espressomd.propagation
lbf = self.lb_class(**self.params, **self.lb_params)
Expand Down
5 changes: 3 additions & 2 deletions testsuite/python/lb_thermostat.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def check_partcl_temp(self, partcl_vel):
partcl_vel.reshape((-1, 3)), vel_range, n_bins, 0.02, KT,
mass=self.partcl_mass)

@utx.skipIfMissingFeatures(["MASS"])
@utx.skipIfMissingFeatures(["MASS", "THERMOSTAT_PER_PARTICLE"])
def test_temperature_with_particles(self):
n_partcls_per_type = 50
partcls_global_gamma = self.system.part.add(
Expand Down Expand Up @@ -140,7 +140,8 @@ def test_temperature_with_particles(self):

np.testing.assert_allclose(fluid_kT, KT, rtol=0.03)

@utx.skipIfMissingFeatures(["EXTERNAL_FORCES"])
@utx.skipIfMissingFeatures(["EXTERNAL_FORCES", "THERMOSTAT_PER_PARTICLE",
"MASS"])
def test_friction(self):
"""apply force and measure if the average velocity matches expectation"""

Expand Down
23 changes: 7 additions & 16 deletions testsuite/python/lees_edwards.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ def setUp(self):
def tearDown(self):
system = self.system
system.part.clear()
system.non_bonded_inter.reset()
system.bonded_inter.clear()
system.lees_edwards.protocol = None
if espressomd.has_features("COLLISION_DETECTION"):
Expand Down Expand Up @@ -745,44 +746,33 @@ def test_le_breaking_bonds(self):
bond_list += p.bonds
np.testing.assert_array_equal(len(bond_list), 0)

const_offset_params = {
'shear_velocity': 0.0,
'shear_direction': 0,
'shear_plane_normal': 1,
'initial_pos_offset': 17.2}

shear_params = {
'shear_velocity': 0.1,
'shear_direction': 0,
'shear_plane_normal': 2,
'initial_pos_offset': -np.sqrt(0.1)}

@utx.skipIfMissingFeatures("LENNARD_JONES")
def run_lj_pair_visibility(self, shear_direction, shear_plane_normal):
"""
Simulate LJ particles coming into contact under linear shear and verify forces.
This is to make sure that no pairs get lost or are outdated
in the short range loop.
"""
assert espressomd.has_features(["LENNARD_JONES"])
shear_axis, normal_axis = axis(
shear_direction), axis(shear_plane_normal)
system = self.system
system.part.clear()
system.time = 0
system.time_step = 0.1
cutoff = 1.5
protocol = espressomd.lees_edwards.LinearShear(
shear_velocity=3, initial_pos_offset=5)
system.lees_edwards.set_boundary_conditions(
shear_direction=shear_direction, shear_plane_normal=shear_plane_normal, protocol=protocol)
system.cell_system.skin = 0.2
system.non_bonded_inter[0, 0].lennard_jones.set_params(
epsilon=1E-6, sigma=1, cutoff=1.2, shift="auto")
epsilon=1E-6, sigma=1, cutoff=cutoff, shift="auto")
system.part.add(
pos=(0.1 * normal_axis, -0.8 * normal_axis),
v=(1.0 * shear_axis, -0.3 * shear_axis))
assert np.all(system.part.all().f == 0.)
tests_common.check_non_bonded_loop_trace(
self, system, cutoff=system.non_bonded_inter[0, 0].lennard_jones.get_params()["cutoff"] + system.cell_system.skin)
self, system, cutoff=cutoff + system.cell_system.skin)

# Rewind the clock to get back the LE offset applied during force calc
system.time = system.time - system.time_step
Expand All @@ -793,11 +783,12 @@ def run_lj_pair_visibility(self, shear_direction, shear_plane_normal):
if np.any(np.abs(system.part.all().f) > 0):
have_interacted = True
tests_common.check_non_bonded_loop_trace(
self, system, cutoff=system.non_bonded_inter[0, 0].lennard_jones.get_params()["cutoff"] + system.cell_system.skin)
self, system, cutoff=cutoff + system.cell_system.skin)
system.time = system.time - system.time_step
tests_common.verify_lj_forces(system, 1E-7)
assert have_interacted

@utx.skipIfMissingFeatures(["LENNARD_JONES"])
def test_zz_lj_pair_visibility(self):
# check that regular decomposition without fully connected doesn't
# catch the particle
Expand Down
2 changes: 1 addition & 1 deletion testsuite/python/save_checkpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@
system.electrostatics.solver = p3m
p3m.charge_neutrality_tolerance = 5e-12

if "ase" in sys.modules:
if has_ase and "ase" in sys.modules:
system.ase = espressomd.plugins.ase.ASEInterface(
type_mapping={0: "H", 1: "O", 10: "Cl"},
)
Expand Down
15 changes: 8 additions & 7 deletions testsuite/python/test_checkpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
and ('LB.CPU' in modes or 'LB.GPU' in modes and is_gpu_available))
has_p3m_mode = 'P3M.CPU' in modes or 'P3M.GPU' in modes and is_gpu_available
has_thermalized_bonds = 'THERM.LB' in modes or 'THERM.LANGEVIN' in modes
has_drude = (espressomd.has_features(['ELECTROSTATICS' and 'MASS', 'ROTATION'])
has_drude = (espressomd.has_features(['ELECTROSTATICS', 'MASS', 'ROTATION'])
and has_thermalized_bonds)
has_ase = 'ASE' in modes

Expand All @@ -65,6 +65,7 @@ class CheckpointTest(ut.TestCase):
checkpoint = espressomd.checkpointing.Checkpoint(
**config.get_checkpoint_params())
checkpoint.load(0)
checkpoint.save(1)
path_cpt_root = pathlib.Path(checkpoint.checkpoint_dir)

@classmethod
Expand All @@ -78,7 +79,7 @@ def setUpClass(cls):
cls.ref_periodicity = np.array([False, False, False])

@utx.skipIfMissingFeatures(["WALBERLA"])
@ut.skipIf(not has_lb_mode, "Skipping test due to missing LB feature.")
@ut.skipIf(not has_lb_mode, "Skipping test due to missing LB mode.")
def test_lb_fluid(self):
lbf = system.lb
cpt_mode = 0 if 'LB.ASCII' in modes else 1
Expand Down Expand Up @@ -164,7 +165,7 @@ def test_lb_fluid(self):
np.copy(lbf[:, :, :].is_boundary.astype(int)), 0)

@utx.skipIfMissingFeatures(["WALBERLA"])
@ut.skipIf(not has_lb_mode, "Skipping test due to missing EK feature.")
@ut.skipIf(not has_lb_mode, "Skipping test due to missing EK mode.")
def test_ek_species(self):
cpt_mode = 0 if 'LB.ASCII' in modes else 1
cpt_root = pathlib.Path(self.checkpoint.checkpoint_dir)
Expand Down Expand Up @@ -269,7 +270,7 @@ def generator(value, shape):

@utx.skipIfMissingFeatures(["WALBERLA"])
@ut.skipIf('LB.GPU' in modes, 'VTK not implemented for LB GPU')
@ut.skipIf(not has_lb_mode, "Skipping test due to missing LB feature.")
@ut.skipIf(not has_lb_mode, "Skipping test due to missing LB mode.")
def test_lb_vtk(self):
lbf = system.lb
self.assertEqual(len(lbf.vtk_writers), 2)
Expand Down Expand Up @@ -316,7 +317,7 @@ def test_lb_vtk(self):
(vtk_root / filename.format(2)).unlink(missing_ok=True)

@utx.skipIfMissingFeatures(["WALBERLA"])
@ut.skipIf(not has_lb_mode, "Skipping test due to missing EK feature.")
@ut.skipIf(not has_lb_mode, "Skipping test due to missing EK mode.")
def test_ek_vtk(self):
ek_species = system.ekcontainer[0]
vtk_suffix = config.test_name
Expand Down Expand Up @@ -507,7 +508,7 @@ def test_shape_based_constraints_serialization(self):
self.assertGreater(np.linalg.norm(np.copy(p3.f) - old_force), 1e6)

@utx.skipIfMissingFeatures(["WALBERLA"])
@ut.skipIf(not has_lb_mode, "Skipping test due to missing LB feature.")
@ut.skipIf(not has_lb_mode, "Skipping test due to missing LB mode.")
@ut.skipIf('THERM.LB' not in modes, 'LB thermostat not in modes')
def test_thermostat_LB(self):
thmst = system.thermostat.lb
Expand Down Expand Up @@ -916,7 +917,7 @@ def test_scafacos_dipoles(self):
self.assertEqual(state[key], reference[key], msg=f'for {key}')

def test_comfixed(self):
self.assertEqual(list(system.comfixed.types), [0, 2])
self.assertEqual(set(system.comfixed.types), {0, 2})

@utx.skipIfMissingFeatures('COLLISION_DETECTION')
def test_collision_detection(self):
Expand Down

0 comments on commit e3ac3af

Please sign in to comment.