From af1f442b97550d7e8228463cf44ce5b69cc83b30 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Tue, 14 Jan 2025 07:06:37 -0500 Subject: [PATCH 01/41] increment bugfix for "inputs local" --- src/compute_reduce.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compute_reduce.cpp b/src/compute_reduce.cpp index 40bb206bd2..6c4bafab4f 100644 --- a/src/compute_reduce.cpp +++ b/src/compute_reduce.cpp @@ -218,7 +218,7 @@ ComputeReduce::ComputeReduce(LAMMPS *lmp, int narg, char **arg) : input_mode = PERATOM; else if (strcmp(arg[iarg + 1], "local") == 0) input_mode = LOCAL; - iarg += 2; + iarg += 1; } else error->all(FLERR, "Unknown compute {} keyword: {}", style, arg[iarg]); } From a4f02fbad37df41709cf20ae84510e9590111ed7 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Tue, 14 Jan 2025 09:56:06 -0500 Subject: [PATCH 02/41] handle the case when pair-wise cutoff varies and neighbor list is not sufficient --- src/compute_rdf.cpp | 49 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/src/compute_rdf.cpp b/src/compute_rdf.cpp index 372bebbfc7..2dcbf6ed39 100644 --- a/src/compute_rdf.cpp +++ b/src/compute_rdf.cpp @@ -38,6 +38,8 @@ using namespace LAMMPS_NS; using namespace MathConst; +static constexpr double BIG = 1.0e20; + /* ---------------------------------------------------------------------- */ ComputeRDF::ComputeRDF(LAMMPS *lmp, int narg, char **arg) : @@ -168,30 +170,63 @@ ComputeRDF::~ComputeRDF() void ComputeRDF::init() { + const double skin = neighbor->skin; if (!force->pair && !cutflag) error->all(FLERR,"Compute rdf requires a pair style or an explicit cutoff"); + // check if the pair style cutoff varies + double pairmaxcut, pairmincut; + if (force->pair) { + pairmaxcut = 0.0; + pairmincut = BIG; + for (int i = 1; i <= atom->ntypes; i++) + for (int j = i; j <= atom->ntypes; j++) { + const double cut = sqrt(force->pair->cutsq[i][j]); + pairmaxcut = MAX(pairmaxcut, cut); + pairmincut = MIN(pairmincut, cut); + } + } else { + pairmaxcut = pairmincut = cutoff_user; + } + if (cutflag) { - double skin = neighbor->skin; mycutneigh = cutoff_user + skin; double cutghost; // as computed by Neighbor and Comm + double cutforce = 0.0; // largest pairwise cutoff plus skin + if (force->pair) cutforce = force->pair->cutforce + skin; if (force->pair) cutghost = MAX(force->pair->cutforce+skin,comm->cutghostuser); else cutghost = comm->cutghostuser; if (mycutneigh > cutghost) - error->all(FLERR,"Compute rdf cutoff exceeds ghost atom range - " - "use comm_modify cutoff command"); - if (force->pair && mycutneigh < force->pair->cutforce + skin) + error->all(FLERR,"Compute rdf cutoff plus skin {} exceeds ghost atom range {} - " + "use comm_modify cutoff command to increase it", mycutneigh, cutghost); + + // if the pair-wise cutoff varies for different pairs of types, the neighbor list code + // will still re-use the pairwise neighbor list if the *largest* cutoff is sufficient. + // this will lead to incorrect results and a larger user cutoff is required. + + if ((cutoff_user > pairmincut) && (cutoff_user <= pairmaxcut)) + error->all(FLERR,"Compute rdf cutoff {} must be larger than the maximum pair-wise cutoff {} " + "when the pair-wise cutoff varies", cutoff_user, pairmaxcut); + + if (force->pair && (cutoff_user < pairmaxcut)) { if (comm->me == 0) - error->warning(FLERR,"Compute rdf cutoff less than neighbor cutoff - " - "forcing a needless neighbor list build"); + error->warning(FLERR,"Compute rdf cutoff {} is less than the pair-wise cutoff {} - " + "forcing a needless neighbor list build", cutoff_user, pairmaxcut); + } delr = cutoff_user / nbin; - } else delr = force->pair->cutforce / nbin; + } else { + if ((pairmincut != pairmaxcut) && (comm->me == 0)) + error->warning(FLERR,"Pair-wise cutoff varies between {} and {}. Individual rdf functions " + "are only correct up to that cutoff - Use cutoff keyword to force a common " + "cutoff", pairmincut, pairmaxcut); + delr = force->pair->cutforce / nbin; + } delrinv = 1.0/delr; From fbc66f75acf90a22ace1ebb75f10a8bab5bf107d Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Tue, 14 Jan 2025 20:46:43 -0500 Subject: [PATCH 03/41] adjust epsilon (again) for macOS on ARM --- unittest/force-styles/tests/manybody-pair-dispersion_d3.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/force-styles/tests/manybody-pair-dispersion_d3.yaml b/unittest/force-styles/tests/manybody-pair-dispersion_d3.yaml index b145ce6ee7..ac0130f9ce 100644 --- a/unittest/force-styles/tests/manybody-pair-dispersion_d3.yaml +++ b/unittest/force-styles/tests/manybody-pair-dispersion_d3.yaml @@ -1,7 +1,7 @@ --- lammps_version: 19 Nov 2024 date_generated: Wed Dec 11 15:29:39 2024 -epsilon: 1e-7 +epsilon: 1e-6 skip_tests: prerequisites: ! | pair dispersion/d3 From 781b40643c570338b1424b21a08213d8a25f970c Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Tue, 14 Jan 2025 20:47:23 -0500 Subject: [PATCH 04/41] add deprecation warning for using accelerator offload with INTEL package --- cmake/Modules/Packages/INTEL.cmake | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmake/Modules/Packages/INTEL.cmake b/cmake/Modules/Packages/INTEL.cmake index e6755bf23b..6fb1c57e8a 100644 --- a/cmake/Modules/Packages/INTEL.cmake +++ b/cmake/Modules/Packages/INTEL.cmake @@ -72,6 +72,10 @@ if(INTEL_ARCH STREQUAL "KNL") if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "Intel") message(FATAL_ERROR "Must use Intel compiler with INTEL for KNL architecture") endif() + message(WARNING, "Support for Intel Xeon Phi accelerators and Knight's Landing CPUs " + "will be removed from LAMMPS in Summer 2025 due to lack of available machines " + "in labs and HPC centers and removed support in recent compilers " + "Please contact developers@lammps.org if you have any concerns about this step.") set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -xHost -qopenmp -qoffload") set(MIC_OPTIONS "-qoffload-option,mic,compiler,\"-fp-model fast=2 -mGLOB_default_function_attrs=\\\"gather_scatter_loop_unroll=4\\\"\"") target_compile_options(lammps PRIVATE -xMIC-AVX512 -qoffload -fno-alias -ansi-alias -restrict -qoverride-limits ${MIC_OPTIONS}) From 5f502782411a8571fea1a1463284b9eefec323a0 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Tue, 14 Jan 2025 22:06:00 -0500 Subject: [PATCH 05/41] transfer workaround from compute rdf --- src/EXTRA-COMPUTE/compute_adf.cpp | 39 +++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/EXTRA-COMPUTE/compute_adf.cpp b/src/EXTRA-COMPUTE/compute_adf.cpp index 20b1749fa9..2cbecbec99 100644 --- a/src/EXTRA-COMPUTE/compute_adf.cpp +++ b/src/EXTRA-COMPUTE/compute_adf.cpp @@ -39,6 +39,8 @@ using MathConst::RAD2DEG; enum { DEGREE, RADIAN, COSINE }; +static constexpr double BIG = 1.0e20; + /* ---------------------------------------------------------------------- compute angular distribution functions for I, J, K atoms ---------------------------------------------------------------------- */ @@ -133,15 +135,15 @@ ComputeADF::ComputeADF(LAMMPS *lmp, int narg, char **arg) : utils::bounds(FLERR,arg[iarg+1],1,atom->ntypes,jlo[m],jhi[m],error); utils::bounds(FLERR,arg[iarg+2],1,atom->ntypes,klo[m],khi[m],error); if ((ilo[m] > ihi[m]) || (jlo[m] > jhi[m]) || (klo[m] > khi[m])) - error->all(FLERR,"Illegal compute adf command"); + error->all(FLERR,"Illegal compute adf command index range"); rcutinnerj[m] = utils::numeric(FLERR,arg[iarg+3],false,lmp); rcutouterj[m] = utils::numeric(FLERR,arg[iarg+4],false,lmp); if (rcutinnerj[m] < 0.0 || rcutinnerj[m] >= rcutouterj[m]) - error->all(FLERR,"Illegal compute adf command"); + error->all(FLERR,"Illegal compute adf command j-cutoff"); rcutinnerk[m] = utils::numeric(FLERR,arg[iarg+5],false,lmp); rcutouterk[m] = utils::numeric(FLERR,arg[iarg+6],false,lmp); if (rcutinnerk[m] < 0.0 || rcutinnerk[m] >= rcutouterk[m]) - error->all(FLERR,"Illegal compute adf command"); + error->all(FLERR,"Illegal compute adf command k-cutoff"); iarg += nargsperadf; } } @@ -290,8 +292,8 @@ void ComputeADF::init() double skin = neighbor->skin; mycutneigh = maxouter + skin; if (mycutneigh > comm->cutghostuser) - error->all(FLERR,"Compute adf outer cutoff exceeds ghost atom range - " - "use comm_modify cutoff command"); + error->all(FLERR,"Compute adf outer cutoff {} exceeds ghost atom range {} - " + "use comm_modify cutoff command", mycutneigh, comm->cutghostuser); } // assign ordinate values to 1st column of output array @@ -328,6 +330,33 @@ void ComputeADF::init() if (mycutneigh > 0.0) { if ((neighbor->style == Neighbor::MULTI) || (neighbor->style == Neighbor::MULTI_OLD)) error->all(FLERR, "Compute adf with custom cutoffs requires neighbor style 'bin' or 'nsq'"); + + // check if the pair style cutoff varies + double pairmaxcut, pairmincut; + double skin = neighbor->skin; + double cutoff_user = mycutneigh - skin; + + if (force->pair) { + pairmaxcut = 0.0; + pairmincut = BIG; + for (int i = 1; i <= atom->ntypes; i++) + for (int j = i; j <= atom->ntypes; j++) { + const double cut = sqrt(force->pair->cutsq[i][j]); + pairmaxcut = MAX(pairmaxcut, cut); + pairmincut = MIN(pairmincut, cut); + } + } else { + pairmaxcut = pairmincut = cutoff_user; + } + + // if the pair-wise cutoff varies for different pairs of types, the neighbor list code + // will still re-use the pairwise neighbor list if the *largest* cutoff is sufficient. + // this will lead to incorrect results and a larger user cutoff is required. + + if ((cutoff_user > pairmincut) && (cutoff_user <= pairmaxcut)) + error->all(FLERR,"Compute adf max cutoff {} must be larger than the maximum pair-wise " + "cutoff {} when the pair-wise cutoff varies", cutoff_user, pairmaxcut); + req->set_cutoff(mycutneigh); } } From 2cbdaf8a6a2e6f497faf3962ef6fb4f375cc11ed Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Tue, 14 Jan 2025 22:23:24 -0500 Subject: [PATCH 06/41] fix some issues flagged by coverity scan --- src/MISC/fix_imd.cpp | 5 ++--- src/SHOCK/fix_append_atoms.cpp | 9 +++++---- src/VORONOI/compute_voronoi_atom.cpp | 7 +------ src/respa.cpp | 2 ++ 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/MISC/fix_imd.cpp b/src/MISC/fix_imd.cpp index 0dbafe49cb..04adcabd34 100644 --- a/src/MISC/fix_imd.cpp +++ b/src/MISC/fix_imd.cpp @@ -581,9 +581,8 @@ FixIMD::FixIMD(LAMMPS *lmp, int narg, char **arg) : msglen += 3*4*num_coords+IMDHEADERSIZE; } msgdata = new char[msglen]; - } - else { - msglen = 3*sizeof(float)*num_coords+IMDHEADERSIZE; + } else { + msglen = 3*(int)sizeof(float)*num_coords+IMDHEADERSIZE; msgdata = new char[msglen]; } diff --git a/src/SHOCK/fix_append_atoms.cpp b/src/SHOCK/fix_append_atoms.cpp index 677b3b55fd..3f15d13df2 100644 --- a/src/SHOCK/fix_append_atoms.cpp +++ b/src/SHOCK/fix_append_atoms.cpp @@ -49,7 +49,6 @@ FixAppendAtoms::FixAppendAtoms(LAMMPS *lmp, int narg, char **arg) : scaleflag = 1; spatflag=0; - spatialid = nullptr; size = 0.0; xloflag = xhiflag = yloflag = yhiflag = zloflag = zhiflag = 0; @@ -60,9 +59,6 @@ FixAppendAtoms::FixAppendAtoms(LAMMPS *lmp, int narg, char **arg) : rany = 0.0; ranz = 0.0; - randomx = nullptr; - randomt = nullptr; - if (domain->lattice->nbasis == 0) error->all(FLERR,"Fix append/atoms requires a lattice be defined"); @@ -123,6 +119,7 @@ FixAppendAtoms::FixAppendAtoms(LAMMPS *lmp, int narg, char **arg) : if (strcmp(arg[iarg+1],"f_") == 0) error->all(FLERR, "Bad fix ID in fix append/atoms command"); spatflag = 1; + delete[] spatialid; spatialid = utils::strdup(arg[iarg+1]+2); spatlead = utils::numeric(FLERR,arg[iarg+2],false,lmp); iarg += 3; @@ -152,6 +149,7 @@ FixAppendAtoms::FixAppendAtoms(LAMMPS *lmp, int narg, char **arg) : ranz = utils::numeric(FLERR,arg[iarg+3],false,lmp); xseed = utils::inumeric(FLERR,arg[iarg+4],false,lmp); if (xseed <= 0) error->all(FLERR,"Illegal fix append/atoms command"); + delete randomx; randomx = new RanMars(lmp,xseed + comm->me); iarg += 5; } else if (strcmp(arg[iarg],"temp") == 0) { @@ -165,7 +163,10 @@ FixAppendAtoms::FixAppendAtoms(LAMMPS *lmp, int narg, char **arg) : if (t_period <= 0) error->all(FLERR,"Illegal fix append/atoms command"); if (t_extent <= 0) error->all(FLERR,"Illegal fix append/atoms command"); if (tseed <= 0) error->all(FLERR,"Illegal fix append/atoms command"); + delete randomt; randomt = new RanMars(lmp,tseed + comm->me); + delete[] gfactor1; + delete[] gfactor2; gfactor1 = new double[atom->ntypes+1]; gfactor2 = new double[atom->ntypes+1]; iarg += 5; diff --git a/src/VORONOI/compute_voronoi_atom.cpp b/src/VORONOI/compute_voronoi_atom.cpp index 4aa6ebf559..12ea173a23 100644 --- a/src/VORONOI/compute_voronoi_atom.cpp +++ b/src/VORONOI/compute_voronoi_atom.cpp @@ -55,16 +55,10 @@ ComputeVoronoi::ComputeVoronoi(LAMMPS *lmp, int narg, char **arg) : surface = VOROSURF_NONE; maxedge = 0; fthresh = ethresh = 0.0; - radstr = nullptr; onlyGroup = false; occupation = false; - con_mono = nullptr; - con_poly = nullptr; - tags = nullptr; oldmaxtag = 0; - occvec = sendocc = lroot = lnext = nullptr; - faces = nullptr; int iarg = 3; while (iarg narg || strstr(arg[iarg+1],"v_") != arg[iarg+1] ) error->all(FLERR,"Illegal compute voronoi/atom command"); + delete[] radstr; radstr = utils::strdup(&arg[iarg+1][2]); iarg += 2; } diff --git a/src/respa.cpp b/src/respa.cpp index fb54582553..086371ecbb 100644 --- a/src/respa.cpp +++ b/src/respa.cpp @@ -125,6 +125,8 @@ Respa::Respa(LAMMPS *lmp, int narg, char **arg) : nhybrid_styles = hybrid->nstyles; // each hybrid sub-style needs to be assigned to a respa level if (iarg + nhybrid_styles > narg) error->all(FLERR, "Illegal run_style respa command"); + delete[] hybrid_level; + delete[] hybrid_compute; hybrid_level = new int[nhybrid_styles]; hybrid_compute = new int[nhybrid_styles]; for (int i = 0; i < nhybrid_styles; ++i) { From 4afdf493d7f50a16b26220648efa9d5dab0ca9f3 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Wed, 15 Jan 2025 06:05:25 -0500 Subject: [PATCH 07/41] integrate and adapt contents from PR #4028 by @alphataubio --- doc/src/Speed_kokkos.rst | 80 +++++++++++++++++++++++++++++++++++----- 1 file changed, 70 insertions(+), 10 deletions(-) diff --git a/doc/src/Speed_kokkos.rst b/doc/src/Speed_kokkos.rst index 9f8dcf8340..f8379949a4 100644 --- a/doc/src/Speed_kokkos.rst +++ b/doc/src/Speed_kokkos.rst @@ -67,6 +67,14 @@ version 23 November 2023 and Kokkos version 4.2. To build with Kokkos support for AMD GPUs, the AMD ROCm toolkit software version 5.2.0 or later must be installed on your system. +.. admonition:: Intel Data Center GPU support + :class: note + + Support for Kokkos with Intel Data Center GPU accelerators (formerly + known under the code name "Ponte Vecchio") in LAMMPS is still a work + in progress. Only a subset of the functionality works correctly. + Please contact the LAMMPS developers if you run into problems. + .. admonition:: CUDA and MPI library compatibility :class: note @@ -80,13 +88,15 @@ version 23 November 2023 and Kokkos version 4.2. LAMMPS command-line or by using the command :doc:`package kokkos gpu/aware off ` in the input file. -.. admonition:: Intel Data Center GPU support +.. admonition:: Using multiple MPI ranks per GPU :class: note - Support for Kokkos with Intel Data Center GPU accelerators (formerly - known under the code name "Ponte Vecchio") in LAMMPS is still a work - in progress. Only a subset of the functionality works correctly. - Please contact the LAMMPS developers if you run into problems. + Unlike with the GPU package, there are limited benefits from using + multiple MPI processes per GPU with KOKKOS. But when doing this it + is **required** to enable CUDA MPS (`Multi-Process Service :: GPU + Deployment and Management Documentation + `_ ) to get acceptable + performance. Building LAMMPS with the KOKKOS package """"""""""""""""""""""""""""""""""""""" @@ -365,13 +375,13 @@ one or more nodes, each with two GPUs: .. note:: - When using a GPU, you will achieve the best performance if your - input script does not use fix or compute styles which are not yet + When using a GPU, you will achieve the best performance if your input + script does not use fix or compute styles which are not yet Kokkos-enabled. This allows data to stay on the GPU for multiple timesteps, without being copied back to the host CPU. Invoking a - non-Kokkos fix or compute, or performing I/O for - :doc:`thermo ` or :doc:`dump ` output will cause data - to be copied back to the CPU incurring a performance penalty. + non-Kokkos fix or compute, or performing I/O for :doc:`thermo + ` or :doc:`dump ` output will cause data to be + copied back to the CPU incurring a performance penalty. .. note:: @@ -379,6 +389,56 @@ one or more nodes, each with two GPUs: kspace, etc., you must set the environment variable ``CUDA_LAUNCH_BLOCKING=1``. However, this will reduce performance and is not recommended for production runs. +Troubleshooting segmentation faults on GPUs +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +As noted above, KOKKOS by default assumes that the MPI library is +GPU-aware. This is not always the case and can lead to segmentation +faults when using more than one MPI process. Normally, LAMMPS will +print a warning like "*Turning off GPU-aware MPI since it is not +detected*", or an error message like "*Kokkos with GPU-enabled backend +assumes GPU-aware MPI is available*", OR a **segmentation fault**. To +confirm that a segmentation fault is caused by this, you can turn off +the GPU-aware assumption via the :doc:`package kokkos command ` +or the corresponding command-line flag. + +If you still get a segmentation fault, despite running with only one MPI +process or using the command-line flag to turn off expecting a GPU-aware +MPI library, then using the CMake compile setting +``-DKokkos_ENABLE_DEBUG=on`` or adding ``KOKKOS_DEBUG=yes`` to your +machine makefile for building with traditional make will generate useful +output that can be passed to the LAMMPS developers for further +debugging. + +Troubleshooting memory allocation on GPUs +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +`Kokkos Tools `_ provides a set +of lightweight profiling and debugging utilities, which interface with +instrumentation hooks (eg. `space-time-stack +`_) +built directly into the Kokkos runtime. After compiling a dynamic LAMMPS +library, you then have to set the environment variable ``KOKKOS_TOOLS_LIBS`` +before executing your LAMMPS Kokkos run. Example: + +.. code-block:: bash + + export KOKKOS_TOOLS_LIBS=${HOME}/kokkos-tools/src/tools/memory-events/kp_memory_event.so + mpirun -np 4 lmp_kokkos_cuda_openmpi -in in.lj -k on g 4 -sf kk + +Starting with the NVIDIA Pascal GPU architecture, CUDA supports +`"Unified Virtual Memory" (UVM) +`_ +which enables allocating more memory than a GPU possesses by also using +memory on the host CPU and then CUDA will transparently move data +between CPU and GPU as needed. The resulting LAMMPS performance depends +on `memory access pattern, data residency, and GPU memory +oversubscription +`_ +. The CMake option ``-DKokkos_ENABLE_CUDA_UVM=on`` or the makefile +setting ``KOKKOS_CUDA_OPTIONS=enable_lambda,force_uvm`` enables using +:ref:`UVM with Kokkos ` when compiling LAMMPS. + Run with the KOKKOS package by editing an input script ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 48f49837d82bf6953c17ed544e3fcb85ca54d50c Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Wed, 15 Jan 2025 08:23:58 -0500 Subject: [PATCH 08/41] fix some more minor memory issues flagged by coverity scan --- src/EXTRA-FIX/fix_ttm_mod.cpp | 30 +++++++++++++++--------------- src/GRANULAR/fix_wall_gran.cpp | 2 ++ src/dump_image.cpp | 17 +++++++++++++---- src/fix_ave_grid.cpp | 26 ++++++++++++-------------- 4 files changed, 42 insertions(+), 33 deletions(-) diff --git a/src/EXTRA-FIX/fix_ttm_mod.cpp b/src/EXTRA-FIX/fix_ttm_mod.cpp index 335acdd853..6ad3ca1b0a 100644 --- a/src/EXTRA-FIX/fix_ttm_mod.cpp +++ b/src/EXTRA-FIX/fix_ttm_mod.cpp @@ -74,14 +74,13 @@ static constexpr double SHIFT = 0.0; /* ---------------------------------------------------------------------- */ FixTTMMod::FixTTMMod(LAMMPS *lmp, int narg, char **arg) : - Fix(lmp, narg, arg), - random(nullptr), gfactor1(nullptr), gfactor2(nullptr), ratio(nullptr), flangevin(nullptr), - T_electron(nullptr), T_electron_old(nullptr), net_energy_transfer(nullptr), - net_energy_transfer_all(nullptr) + Fix(lmp, narg, arg), infile(nullptr), outfile(nullptr), random(nullptr), gfactor1(nullptr), + gfactor2(nullptr), ratio(nullptr), flangevin(nullptr), T_electron(nullptr), + T_electron_old(nullptr), net_energy_transfer(nullptr), net_energy_transfer_all(nullptr) { if (lmp->citeme) lmp->citeme->add(cite_fix_ttm_mod); - if (narg < 8) error->all(FLERR,"Illegal fix ttm/mod command"); + if (narg < 8) utils::missing_cmd_args(FLERR, "fix ttm/mod", error); vector_flag = 1; size_vector = 2; @@ -103,27 +102,29 @@ FixTTMMod::FixTTMMod(LAMMPS *lmp, int narg, char **arg) : int iarg = 8; while (iarg < narg) { if (strcmp(arg[iarg],"set") == 0) { - if (iarg+2 > narg) error->all(FLERR,"Illegal fix ttm/mod command"); + if (iarg+2 > narg) utils::missing_cmd_args(FLERR, "fix ttm/mod set", error); tinit = utils::numeric(FLERR,arg[iarg+1],false,lmp); if (tinit <= 0.0) error->all(FLERR,"Fix ttm/mod initial temperature must be > 0.0"); iarg += 2; } else if (strcmp(arg[iarg],"infile") == 0) { - if (iarg+2 > narg) error->all(FLERR,"Illegal fix ttm/mod command"); + if (iarg+2 > narg) utils::missing_cmd_args(FLERR, "fix ttm/mod infile", error); + delete[] infile; infile = utils::strdup(arg[iarg+1]); iarg += 2; } else if (strcmp(arg[iarg],"outfile") == 0) { - if (iarg+3 > narg) error->all(FLERR,"Illegal fix ttm/mod command"); + if (iarg+3 > narg) utils::missing_cmd_args(FLERR, "fix ttm/mod outfile", error); + delete[] outfile; outevery = utils::inumeric(FLERR,arg[iarg+1],false,lmp); outfile = utils::strdup(arg[iarg+2]); iarg += 3; - } else error->all(FLERR,"Illegal fix ttm/mod command"); + } else error->all(FLERR,"Unknown fix ttm/mod keyword {}", arg[iarg]); } // error check if (seed <= 0) - error->all(FLERR,"Invalid random number seed in fix ttm/mod command"); + error->all(FLERR,"Invalid random number seed {} in fix ttm/mod command", seed); if (nxgrid <= 0 || nygrid <= 0 || nzgrid <= 0) error->all(FLERR,"Fix ttm/mod grid sizes must be > 0"); @@ -152,7 +153,8 @@ FixTTMMod::FixTTMMod(LAMMPS *lmp, int narg, char **arg) : if (v_0 < 0.0) error->all(FLERR,"Fix ttm/mod v_0 must be >= 0.0"); if (ionic_density <= 0.0) error->all(FLERR,"Fix ttm/mod ionic_density must be > 0.0"); if (surface_l < 0) error->all(FLERR,"Surface coordinates must be >= 0"); - if (surface_l >= surface_r) error->all(FLERR, "Left surface coordinate must be less than right surface coordinate"); + if (surface_l >= surface_r) + error->all(FLERR, "Left surface coordinate must be less than right surface coordinate"); // initialize Marsaglia RNG with processor-unique seed @@ -168,10 +170,8 @@ FixTTMMod::FixTTMMod(LAMMPS *lmp, int narg, char **arg) : memory->create(T_electron_old,nzgrid,nygrid,nxgrid,"ttm/mod:T_electron_old"); memory->create(T_electron_first,nzgrid,nygrid,nxgrid,"ttm/mod:T_electron_first"); memory->create(T_electron,nzgrid,nygrid,nxgrid,"ttm/mod:T_electron"); - memory->create(net_energy_transfer,nzgrid,nygrid,nxgrid, - "ttm/mod:net_energy_transfer"); - memory->create(net_energy_transfer_all,nzgrid,nygrid,nxgrid, - "ttm/mod:net_energy_transfer_all"); + memory->create(net_energy_transfer,nzgrid,nygrid,nxgrid,"ttm/mod:net_energy_transfer"); + memory->create(net_energy_transfer_all,nzgrid,nygrid,nxgrid,"ttm/mod:net_energy_transfer_all"); flangevin = nullptr; grow_arrays(atom->nmax); diff --git a/src/GRANULAR/fix_wall_gran.cpp b/src/GRANULAR/fix_wall_gran.cpp index 3336a8a4d7..4832f07849 100644 --- a/src/GRANULAR/fix_wall_gran.cpp +++ b/src/GRANULAR/fix_wall_gran.cpp @@ -163,6 +163,7 @@ FixWallGran::FixWallGran(LAMMPS *lmp, int narg, char **arg) : } else if (strcmp(arg[iarg],"region") == 0) { if (narg < iarg+2) error->all(FLERR,"Illegal fix wall/gran command"); wallstyle = REGION; + delete[] idregion; idregion = utils::strdup(arg[iarg+1]); iarg += 2; // This option is only compatible with fix wall/gran/region @@ -205,6 +206,7 @@ FixWallGran::FixWallGran(LAMMPS *lmp, int narg, char **arg) : } else if (strcmp(arg[iarg],"temperature") == 0) { if (iarg+2 > narg) error->all(FLERR,"Illegal fix wall/gran command"); if (utils::strmatch(arg[iarg+1], "^v_")) { + delete[] tstr; tstr = utils::strdup(arg[iarg+1] + 2); } else { Twall = utils::numeric(FLERR,arg[iarg+1],false,lmp); diff --git a/src/dump_image.cpp b/src/dump_image.cpp index 9610ef4d9a..abf4d366d5 100644 --- a/src/dump_image.cpp +++ b/src/dump_image.cpp @@ -184,12 +184,12 @@ DumpImage::DumpImage(LAMMPS *lmp, int narg, char **arg) : char *id; int igrid,idata,index; - int iflag = - utils::check_grid_reference((char *) "Dump image", - arg[iarg+1],nevery,id, - igrid,idata,index,lmp); + int iflag = utils::check_grid_reference((char *) "Dump image", arg[iarg+1], nevery, id, + igrid,idata,index,lmp); if (iflag < 0) error->all(FLERR,"Invalid grid reference in dump image command"); + delete[] id_grid_compute; + delete[] id_grid_fix; if (iflag == ArgInfo::COMPUTE) id_grid_compute = utils::strdup(id); else if (iflag == ArgInfo::FIX) id_grid_fix = utils::strdup(id); delete[] id; @@ -252,6 +252,7 @@ DumpImage::DumpImage(LAMMPS *lmp, int narg, char **arg) : } else if (strcmp(arg[iarg],"view") == 0) { if (iarg+3 > narg) error->all(FLERR,"Illegal dump image command"); if (utils::strmatch(arg[iarg+1],"^v_")) { + delete[] thetastr; thetastr = utils::strdup(arg[iarg+1]+2); } else { const double theta = utils::numeric(FLERR,arg[iarg+1],false,lmp); @@ -260,6 +261,7 @@ DumpImage::DumpImage(LAMMPS *lmp, int narg, char **arg) : image->theta = DEG2RAD * theta; } if (utils::strmatch(arg[iarg+2],"^v_")) { + delete[] phistr; phistr = utils::strdup(arg[iarg+2]+2); } else { image->phi = DEG2RAD * utils::numeric(FLERR,arg[iarg+2],false,lmp); @@ -272,14 +274,17 @@ DumpImage::DumpImage(LAMMPS *lmp, int narg, char **arg) : else if (strcmp(arg[iarg+1],"d") == 0) cflag = DYNAMIC; else error->all(FLERR,"Illegal dump image command"); if (utils::strmatch(arg[iarg+2],"^v_")) { + delete[] cxstr; cxstr = utils::strdup(arg[iarg+2]+2); cflag = DYNAMIC; } else cx = utils::numeric(FLERR,arg[iarg+2],false,lmp); if (utils::strmatch(arg[iarg+3],"^v_")) { + delete[] cystr; cystr = utils::strdup(arg[iarg+3]+2); cflag = DYNAMIC; } else cy = utils::numeric(FLERR,arg[iarg+3],false,lmp); if (utils::strmatch(arg[iarg+4],"^v_")) { + delete[] czstr; czstr = utils::strdup(arg[iarg+4]+2); cflag = DYNAMIC; } else cz = utils::numeric(FLERR,arg[iarg+4],false,lmp); @@ -288,12 +293,15 @@ DumpImage::DumpImage(LAMMPS *lmp, int narg, char **arg) : } else if (strcmp(arg[iarg],"up") == 0) { if (iarg+4 > narg) error->all(FLERR,"Illegal dump image command"); if (utils::strmatch(arg[iarg+1],"^v_")) { + delete[] upxstr; upxstr = utils::strdup(arg[iarg+1]+2); } else image->up[0] = utils::numeric(FLERR,arg[iarg+1],false,lmp); if (utils::strmatch(arg[iarg+2],"^v_")) { + delete[] upystr; upystr = utils::strdup(arg[iarg+2]+2); } else image->up[1] = utils::numeric(FLERR,arg[iarg+2],false,lmp); if (utils::strmatch(arg[iarg+3],"^v_")) { + delete[] upzstr; upzstr = utils::strdup(arg[iarg+3]+2); } else image->up[2] = utils::numeric(FLERR,arg[iarg+3],false,lmp); iarg += 4; @@ -301,6 +309,7 @@ DumpImage::DumpImage(LAMMPS *lmp, int narg, char **arg) : } else if (strcmp(arg[iarg],"zoom") == 0) { if (iarg+2 > narg) error->all(FLERR,"Illegal dump image command"); if (utils::strmatch(arg[iarg+1],"^v_")) { + delete[] zoomstr; zoomstr = utils::strdup(arg[iarg+1]+2); } else { double zoom = utils::numeric(FLERR,arg[iarg+1],false,lmp); diff --git a/src/fix_ave_grid.cpp b/src/fix_ave_grid.cpp index 1b69c5644c..471d4191e7 100644 --- a/src/fix_ave_grid.cpp +++ b/src/fix_ave_grid.cpp @@ -42,19 +42,20 @@ enum{DISCARD,KEEP}; static constexpr int OFFSET = 16384; +// clang-format on /* ---------------------------------------------------------------------- */ FixAveGrid::FixAveGrid(LAMMPS *lmp, int narg, char **arg) : - Fix(lmp, narg, arg), id_bias(nullptr), which(nullptr), argindex(nullptr), ids(nullptr), - value2index(nullptr), value2grid(nullptr), value2data(nullptr), grid2d(nullptr), grid3d(nullptr), - grid_buf1(nullptr), grid_buf2(nullptr), grid_output(nullptr), grid_sample(nullptr), - grid_nfreq(nullptr), grid_running(nullptr), grid_window(nullptr), grid2d_previous(nullptr), - grid3d_previous(nullptr), grid_sample_previous(nullptr), grid_nfreq_previous(nullptr), - grid_running_previous(nullptr), grid_window_previous(nullptr), bin(nullptr), skip(nullptr), - vresult(nullptr) + Fix(lmp, narg, arg), id_bias(nullptr), which(nullptr), argindex(nullptr), ids(nullptr), + value2index(nullptr), value2grid(nullptr), value2data(nullptr), grid2d(nullptr), + grid3d(nullptr), grid_buf1(nullptr), grid_buf2(nullptr), grid_output(nullptr), + grid_sample(nullptr), grid_nfreq(nullptr), grid_running(nullptr), grid_window(nullptr), + grid2d_previous(nullptr), grid3d_previous(nullptr), grid_sample_previous(nullptr), + grid_nfreq_previous(nullptr), grid_running_previous(nullptr), grid_window_previous(nullptr), + bin(nullptr), skip(nullptr), vresult(nullptr) { - if (narg < 10) utils::missing_cmd_args(FLERR,"fix ave/grid", error); - + if (narg < 10) utils::missing_cmd_args(FLERR, "fix ave/grid", error); + // clang-format off pergrid_flag = 1; nevery = utils::inumeric(FLERR,arg[3],false,lmp); nrepeat = utils::inumeric(FLERR,arg[4],false,lmp); @@ -193,7 +194,6 @@ FixAveGrid::FixAveGrid(LAMMPS *lmp, int narg, char **arg) : aveflag = ONE; nwindow = 0; biasflag = 0; - id_bias = nullptr; adof = domain->dimension; cdof = 0.0; @@ -231,6 +231,7 @@ FixAveGrid::FixAveGrid(LAMMPS *lmp, int narg, char **arg) : if (iarg+2 > nargnew) error->all(FLERR,"Illegal fix ave/grid command"); biasflag = 1; + delete[] id_bias; id_bias = utils::strdup(arg[iarg+1]); iarg += 2; @@ -347,11 +348,7 @@ FixAveGrid::FixAveGrid(LAMMPS *lmp, int narg, char **arg) : // vresult for per-atom variable evaluation maxatom = 0; - bin = nullptr; - skip = nullptr; - maxvar = 0; - vresult = nullptr; // nvalid = next step on which end_of_step does something // add nvalid to all computes that store invocation times @@ -372,6 +369,7 @@ FixAveGrid::~FixAveGrid() delete[] argindex; for (int m = 0; m < nvalues; m++) delete[] ids[m]; delete[] ids; + delete[] id_bias; delete[] value2index; delete[] value2grid; delete[] value2data; From 8c93986e47308d04df6c4f3846fc045795746e97 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Wed, 15 Jan 2025 23:10:50 -0500 Subject: [PATCH 09/41] add overloads for Error::all() and Error::one() that can point out the location of a faulty argument --- src/create_atoms.cpp | 6 +++--- src/error.cpp | 31 ++++++++++++++++++---------- src/error.h | 49 ++++++++++++++++++++++++++++++++++++-------- src/input.h | 2 +- src/utils.cpp | 22 ++++++++++++++++++++ src/utils.h | 10 +++++++++ 6 files changed, 96 insertions(+), 24 deletions(-) diff --git a/src/create_atoms.cpp b/src/create_atoms.cpp index ade6bdc3c1..b5e72393d3 100644 --- a/src/create_atoms.cpp +++ b/src/create_atoms.cpp @@ -103,7 +103,7 @@ void CreateAtoms::command(int narg, char **arg) style = REGION; if (narg < 3) utils::missing_cmd_args(FLERR, "create_atoms region", error); region = domain->get_region_by_id(arg[2]); - if (!region) error->all(FLERR, "Create_atoms region {} does not exist", arg[2]); + if (!region) error->all(FLERR, 2, "Create_atoms region {} does not exist", arg[2]); region->init(); region->prematch(); iarg = 3; @@ -127,7 +127,7 @@ void CreateAtoms::command(int narg, char **arg) region = nullptr; else { region = domain->get_region_by_id(arg[4]); - if (!region) error->all(FLERR, "Create_atoms region {} does not exist", arg[4]); + if (!region) error->all(FLERR, 4, "Create_atoms region {} does not exist", arg[4]); region->init(); region->prematch(); } @@ -138,7 +138,7 @@ void CreateAtoms::command(int narg, char **arg) meshfile = arg[2]; iarg = 3; } else - error->all(FLERR, "Unknown create_atoms command option {}", arg[1]); + error->all(FLERR, 1, "Unknown create_atoms command option {}", arg[1]); // process optional keywords diff --git a/src/error.cpp b/src/error.cpp index e591091b35..1d656c1c45 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -114,7 +114,7 @@ void Error::universe_warn(const std::string &file, int line, const std::string & force MPI_Abort if running in multi-partition mode ------------------------------------------------------------------------- */ -void Error::all(const std::string &file, int line, const std::string &str) +void Error::all(const std::string &file, int line, int failed, const std::string &str) { MPI_Barrier(world); @@ -125,9 +125,11 @@ void Error::all(const std::string &file, int line, const std::string &str) if (me == 0) { std::string mesg = "ERROR: " + str; + if (input && input->line) lastcmd = input->line; try { mesg += fmt::format(" ({}:{})\nLast command: {}\n", truncpath(file),line,lastcmd); + if (failed > NOPOINTER) mesg += utils::point_to_error(input, failed); } catch (fmt::format_error &) { ; // do nothing } @@ -140,6 +142,7 @@ void Error::all(const std::string &file, int line, const std::string &str) if (update) update->whichflag = 0; std::string msg = fmt::format("ERROR: {} ({}:{})\n", str, truncpath(file), line); + if (failed > NOPOINTER) msg += utils::point_to_error(input, failed); if (universe->nworlds > 1) throw LAMMPSAbortException(msg, universe->uworld); @@ -154,15 +157,21 @@ void Error::all(const std::string &file, int line, const std::string &str) forces abort of entire world (and universe) if any proc in world calls ------------------------------------------------------------------------- */ -void Error::one(const std::string &file, int line, const std::string &str) +void Error::one(const std::string &file, int line, int failed, const std::string &str) { int me; std::string lastcmd = "(unknown)"; MPI_Comm_rank(world,&me); if (input && input->line) lastcmd = input->line; - std::string mesg = fmt::format("ERROR on proc {}: {} ({}:{})\nLast command: {}\n", - me,str,truncpath(file),line,lastcmd); + std::string mesg; + try { + mesg = fmt::format("ERROR on proc {}: {} ({}:{})\nLast command: {}\n", + me,str,truncpath(file),line,lastcmd); + if (failed > NOPOINTER) mesg += utils::point_to_error(input, failed); + } catch (fmt::format_error &) { + ; // do nothing + } utils::logmesg(lmp,mesg); if (universe->nworlds > 1) @@ -177,27 +186,27 @@ void Error::one(const std::string &file, int line, const std::string &str) } /* ---------------------------------------------------------------------- - forward vararg version to single string version + forward vararg versions to single string version ------------------------------------------------------------------------- */ -void Error::_all(const std::string &file, int line, fmt::string_view format, +void Error::_all(const std::string &file, int line, int failed, fmt::string_view format, fmt::format_args args) { try { - all(file,line,fmt::vformat(format, args)); + all(file, line, failed, fmt::vformat(format, args)); } catch (fmt::format_error &e) { - all(file,line,e.what()); + all(file, line, NOPOINTER, e.what()); } exit(1); // to trick "smart" compilers into believing this does not return } -void Error::_one(const std::string &file, int line, fmt::string_view format, +void Error::_one(const std::string &file, int line, int failed, fmt::string_view format, fmt::format_args args) { try { - one(file,line,fmt::vformat(format, args)); + one(file, line, failed, fmt::vformat(format, args)); } catch (fmt::format_error &e) { - one(file,line,e.what()); + one(file, line, NOPOINTER, e.what()); } exit(1); // to trick "smart" compilers into believing this does not return } diff --git a/src/error.h b/src/error.h index 805bd4cd0d..cf066338a4 100644 --- a/src/error.h +++ b/src/error.h @@ -27,18 +27,49 @@ class Error : protected Pointers { [[noreturn]] void universe_one(const std::string &, int, const std::string &); void universe_warn(const std::string &, int, const std::string &); - [[noreturn]] void all(const std::string &, int, const std::string &); - template - [[noreturn]] void all(const std::string &file, int line, const std::string &format, Args &&...args) + static constexpr int NOPOINTER = -2; + + // regular error calls + + [[noreturn]] void all(const std::string &file, int line, const std::string &str) { - _all(file, line, format, fmt::make_format_args(args...)); + all(file, line, NOPOINTER, str); } - [[noreturn]] void one(const std::string &, int, const std::string &); template - [[noreturn]] void one(const std::string &file, int line, const std::string &format, Args &&...args) + [[noreturn]] void all(const std::string &file, int line, const std::string &format, + Args &&...args) { - _one(file, line, format, fmt::make_format_args(args...)); + _all(file, line, NOPOINTER, format, fmt::make_format_args(args...)); + } + + [[noreturn]] void one(const std::string &file, int line, const std::string &str) + { + one(file, line, NOPOINTER, str); + } + + template + [[noreturn]] void one(const std::string &file, int line, const std::string &format, + Args &&...args) + { + _one(file, line, NOPOINTER, format, fmt::make_format_args(args...)); + } + + // overloaded error calls indicating faulty argument in command line + [[noreturn]] void all(const std::string &, int, int, const std::string &); + template + [[noreturn]] void all(const std::string &file, int line, int failed, const std::string &format, + Args &&...args) + { + _all(file, line, failed, format, fmt::make_format_args(args...)); + } + + [[noreturn]] void one(const std::string &, int, int, const std::string &); + template + [[noreturn]] void one(const std::string &file, int line, int failed, const std::string &format, + Args &&...args) + { + _one(file, line, failed, format, fmt::make_format_args(args...)); } void warning(const std::string &, int, const std::string &); @@ -72,8 +103,8 @@ class Error : protected Pointers { int numwarn, maxwarn, allwarn; // internal versions that accept explicit fmtlib arguments - [[noreturn]] void _all(const std::string &, int, fmt::string_view, fmt::format_args args); - [[noreturn]] void _one(const std::string &, int, fmt::string_view, fmt::format_args args); + [[noreturn]] void _all(const std::string &, int, int, fmt::string_view, fmt::format_args args); + [[noreturn]] void _one(const std::string &, int, int, fmt::string_view, fmt::format_args args); void _warning(const std::string &, int, fmt::string_view, fmt::format_args args); void _message(const std::string &, int, fmt::string_view, fmt::format_args args); }; diff --git a/src/input.h b/src/input.h index fe6cf15407..e71ee22174 100644 --- a/src/input.h +++ b/src/input.h @@ -28,6 +28,7 @@ class Input : protected Pointers { friend class SimpleCommandsTest_Echo_Test; public: + char *command; // ptr to current command int narg; // # of command args char **arg; // parsed args for command class Variable *variable; // defined variables @@ -42,7 +43,6 @@ class Input : protected Pointers { int get_jump_skip() const { return jump_skip; } protected: - char *command; // ptr to current command int echo_screen; // 0 = no, 1 = yes int echo_log; // 0 = no, 1 = yes diff --git a/src/utils.cpp b/src/utils.cpp index 0f5b50baf2..000baf88d0 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -132,6 +132,28 @@ void utils::missing_cmd_args(const std::string &file, int line, const std::strin if (error) error->all(file, line, "Illegal {} command: missing argument(s)", cmd); } +std::string utils::point_to_error(Input *input, int failedarg) +{ + if (input) { + std::string cmdline = "Preprocessed: "; + int indicator = cmdline.size(); // error indicator points to command by default + cmdline += input->command; + cmdline += ' '; + + // assemble pre-processed command line and update error indicator position, if needed. + for (int i = 0; i < input->narg; ++i) { + if (i == failedarg) indicator = cmdline.size(); + cmdline += input->arg[i]; + cmdline += ' '; + } + // construct and append error indicator line + cmdline += '\n'; + cmdline += std::string(indicator, ' '); + cmdline += "^\n"; + return cmdline; + } else return std::string("(Failed command line text not available)"); +} + /* specialization for the case of just a single string argument */ void utils::logmesg(LAMMPS *lmp, const std::string &mesg) diff --git a/src/utils.h b/src/utils.h index 1ed514cca4..78e59ab2c8 100644 --- a/src/utils.h +++ b/src/utils.h @@ -28,6 +28,7 @@ namespace LAMMPS_NS { // forward declarations class Error; +class Input; class LAMMPS; namespace utils { @@ -59,6 +60,15 @@ namespace utils { void missing_cmd_args(const std::string &file, int line, const std::string &cmd, Error *error); + /*! Create string with last command after pre-processing and pointing to arg with error + * + * This function is a helper function for error messages. It creates + * + * \param input pointer to the Input class instance (for access to last command args) + * \param failedarg index of the faulty argument (-1 to point to the command itself) + * \return string with two lines: the pre-processed command and a '^' pointing to the faulty argument */ + std::string point_to_error(Input *input, int failedarg); + /*! Internal function handling the argument list for logmesg(). */ void fmtargs_logmesg(LAMMPS *lmp, fmt::string_view format, fmt::format_args args); From f60139d374aeba8407caa24fccf427ef57e75800 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Wed, 15 Jan 2025 23:12:39 -0500 Subject: [PATCH 10/41] some more fixes to address coverity scan warnings. --- src/BODY/compute_temp_body.cpp | 1 + src/BROWNIAN/fix_brownian_base.cpp | 24 +++++++++++++++--------- src/fix_addforce.cpp | 2 ++ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/BODY/compute_temp_body.cpp b/src/BODY/compute_temp_body.cpp index 39b2518600..920dd9db00 100644 --- a/src/BODY/compute_temp_body.cpp +++ b/src/BODY/compute_temp_body.cpp @@ -57,6 +57,7 @@ ComputeTempBody::ComputeTempBody(LAMMPS *lmp, int narg, char **arg) : if (strcmp(arg[iarg],"bias") == 0) { if (iarg+2 > narg) utils::missing_cmd_args(FLERR, "compute temp/body bias", error); tempbias = 1; + delete[] id_bias; id_bias = utils::strdup(arg[iarg+1]); iarg += 2; } else if (strcmp(arg[iarg],"dof") == 0) { diff --git a/src/BROWNIAN/fix_brownian_base.cpp b/src/BROWNIAN/fix_brownian_base.cpp index 508ce4d1c6..7c5f5deb8d 100644 --- a/src/BROWNIAN/fix_brownian_base.cpp +++ b/src/BROWNIAN/fix_brownian_base.cpp @@ -33,8 +33,9 @@ using namespace LAMMPS_NS; using namespace FixConst; /* ---------------------------------------------------------------------- */ - -FixBrownianBase::FixBrownianBase(LAMMPS *lmp, int narg, char **arg) : Fix(lmp, narg, arg) +FixBrownianBase::FixBrownianBase(LAMMPS *lmp, int narg, char **arg) : + Fix(lmp, narg, arg), gamma_t_inv(nullptr), gamma_r_inv(nullptr), gamma_t_invsqrt(nullptr), + gamma_r_invsqrt(nullptr), dipole_body(nullptr), rng(nullptr) { time_integrate = 1; @@ -47,18 +48,18 @@ FixBrownianBase::FixBrownianBase(LAMMPS *lmp, int narg, char **arg) : Fix(lmp, n planar_rot_flag = 0; g2 = 0.0; - if (narg < 5) error->all(FLERR, "Illegal fix brownian command."); + if (narg < 5) utils::missing_cmd_args(FLERR, "fix brownian", error); temp = utils::numeric(FLERR, arg[3], false, lmp); - if (temp <= 0) error->all(FLERR, "Fix brownian temp must be > 0."); + if (temp <= 0) error->all(FLERR, "Fix brownian temp must be > 0.0"); seed = utils::inumeric(FLERR, arg[4], false, lmp); - if (seed <= 0) error->all(FLERR, "Fix brownian seed must be > 0."); + if (seed <= 0) error->all(FLERR, "Fix brownian seed must be > 0"); int iarg = 5; while (iarg < narg) { if (strcmp(arg[iarg], "rng") == 0) { - if (narg == iarg + 1) error->all(FLERR, "Illegal fix brownian command."); + if (narg == iarg + 1) utils::missing_cmd_args(FLERR, "fix brownian rng", error); if (strcmp(arg[iarg + 1], "uniform") == 0) { noise_flag = 1; } else if (strcmp(arg[iarg + 1], "gaussian") == 0) { @@ -67,13 +68,14 @@ FixBrownianBase::FixBrownianBase(LAMMPS *lmp, int narg, char **arg) : Fix(lmp, n } else if (strcmp(arg[iarg + 1], "none") == 0) { noise_flag = 0; } else { - error->all(FLERR, "Illegal fix brownian command."); + error->all(FLERR, "Unknown fix brownian rng keyword {}", arg[iarg + 1]); } iarg = iarg + 2; } else if (strcmp(arg[iarg], "dipole") == 0) { - if (narg == iarg + 3) error->all(FLERR, "Illegal fix brownian command."); + if (narg == iarg + 3) utils::missing_cmd_args(FLERR, "fix brownian dipole", error); dipole_flag = 1; + delete[] dipole_body; dipole_body = new double[3]; dipole_body[0] = utils::numeric(FLERR, arg[iarg + 1], false, lmp); @@ -82,9 +84,11 @@ FixBrownianBase::FixBrownianBase(LAMMPS *lmp, int narg, char **arg) : Fix(lmp, n iarg = iarg + 4; } else if (strcmp(arg[iarg], "gamma_t_eigen") == 0) { - if (narg == iarg + 3) error->all(FLERR, "Illegal fix brownian command."); + if (narg == iarg + 3) utils::missing_cmd_args(FLERR, "fix brownian gamma_t_eigen", error); gamma_t_eigen_flag = 1; + delete[] gamma_t_inv; + delete[] gamma_t_invsqrt; gamma_t_inv = new double[3]; gamma_t_invsqrt = new double[3]; gamma_t_inv[0] = 1. / utils::numeric(FLERR, arg[iarg + 1], false, lmp); @@ -111,6 +115,8 @@ FixBrownianBase::FixBrownianBase(LAMMPS *lmp, int narg, char **arg) : Fix(lmp, n if (narg == iarg + 3) error->all(FLERR, "Illegal fix brownian command."); gamma_r_eigen_flag = 1; + delete[] gamma_r_inv; + delete[] gamma_r_invsqrt; gamma_r_inv = new double[3]; gamma_r_invsqrt = new double[3]; diff --git a/src/fix_addforce.cpp b/src/fix_addforce.cpp index 4920d57f4a..8625080ee9 100644 --- a/src/fix_addforce.cpp +++ b/src/fix_addforce.cpp @@ -85,11 +85,13 @@ FixAddForce::FixAddForce(LAMMPS *lmp, int narg, char **arg) : if (iarg + 2 > narg) utils::missing_cmd_args(FLERR, "fix addforce region", error); region = domain->get_region_by_id(arg[iarg + 1]); if (!region) error->all(FLERR, "Region {} for fix addforce does not exist", arg[iarg + 1]); + delete[] idregion; idregion = utils::strdup(arg[iarg + 1]); iarg += 2; } else if (strcmp(arg[iarg], "energy") == 0) { if (iarg + 2 > narg) utils::missing_cmd_args(FLERR, "fix addforce energy", error); if (utils::strmatch(arg[iarg + 1], "^v_")) { + delete[] estr; estr = utils::strdup(arg[iarg + 1] + 2); } else error->all(FLERR, "Invalid fix addforce energy argument: {}", arg[iarg + 1]); From 36dcb294b3de88404473604003fab18f65c48c32 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 16 Jan 2025 00:23:47 -0500 Subject: [PATCH 11/41] we need tighter checks, also on the smallest pairwise cutoff, before we can re-use a default neighbor list for an occasional list with an explicit cutoff --- src/neighbor.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/neighbor.cpp b/src/neighbor.cpp index c43f86e7ed..090fee9e5e 100644 --- a/src/neighbor.cpp +++ b/src/neighbor.cpp @@ -139,6 +139,7 @@ pairclass(nullptr), pairnames(nullptr), pairmasks(nullptr) ago = -1; cutneighmax = 0.0; + cutneighmin = BIG; cutneighsq = nullptr; cutneighghostsq = nullptr; cuttype = nullptr; @@ -1186,6 +1187,7 @@ void Neighbor::morph_unique() irq = requests[i]; // if cut flag set by requestor and cutoff is different than default, + // or pair-wise cutoff is different for different pairs of atoms types // set unique flag, otherwise unset cut flag // this forces Pair,Stencil,Bin styles to be instantiated separately // also add skin to cutoff of perpetual lists @@ -1194,7 +1196,7 @@ void Neighbor::morph_unique() if (!irq->occasional) irq->cutoff += skin; - if (irq->cutoff != cutneighmax) { + if ((irq->cutoff != cutneighmax) || (cutneighmin != cutneighmax)) { irq->unique = 1; } else { irq->cut = 0; @@ -1510,6 +1512,10 @@ void Neighbor::morph_copy_trim() if (jrq->copy && jrq->copylist == i) continue; + // cannot copy or trim if some pair-wise cutoffs are too small + + if (irq->cut && !jrq->cut && (irq->cutoff > cutneighmin)) continue; + // trim a list with longer cutoff if (irq->cut) icut = irq->cutoff; From 00f23d48295ccb094a849b93432324ed8e805ba9 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 16 Jan 2025 00:38:31 -0500 Subject: [PATCH 12/41] revert workarounds in compute rdf and adf now that the issue is fixed at the root --- src/EXTRA-COMPUTE/compute_adf.cpp | 26 ----------------- src/compute_rdf.cpp | 47 ++----------------------------- 2 files changed, 3 insertions(+), 70 deletions(-) diff --git a/src/EXTRA-COMPUTE/compute_adf.cpp b/src/EXTRA-COMPUTE/compute_adf.cpp index 2cbecbec99..108e03e371 100644 --- a/src/EXTRA-COMPUTE/compute_adf.cpp +++ b/src/EXTRA-COMPUTE/compute_adf.cpp @@ -331,32 +331,6 @@ void ComputeADF::init() if ((neighbor->style == Neighbor::MULTI) || (neighbor->style == Neighbor::MULTI_OLD)) error->all(FLERR, "Compute adf with custom cutoffs requires neighbor style 'bin' or 'nsq'"); - // check if the pair style cutoff varies - double pairmaxcut, pairmincut; - double skin = neighbor->skin; - double cutoff_user = mycutneigh - skin; - - if (force->pair) { - pairmaxcut = 0.0; - pairmincut = BIG; - for (int i = 1; i <= atom->ntypes; i++) - for (int j = i; j <= atom->ntypes; j++) { - const double cut = sqrt(force->pair->cutsq[i][j]); - pairmaxcut = MAX(pairmaxcut, cut); - pairmincut = MIN(pairmincut, cut); - } - } else { - pairmaxcut = pairmincut = cutoff_user; - } - - // if the pair-wise cutoff varies for different pairs of types, the neighbor list code - // will still re-use the pairwise neighbor list if the *largest* cutoff is sufficient. - // this will lead to incorrect results and a larger user cutoff is required. - - if ((cutoff_user > pairmincut) && (cutoff_user <= pairmaxcut)) - error->all(FLERR,"Compute adf max cutoff {} must be larger than the maximum pair-wise " - "cutoff {} when the pair-wise cutoff varies", cutoff_user, pairmaxcut); - req->set_cutoff(mycutneigh); } } diff --git a/src/compute_rdf.cpp b/src/compute_rdf.cpp index 2dcbf6ed39..0dc7da3460 100644 --- a/src/compute_rdf.cpp +++ b/src/compute_rdf.cpp @@ -38,8 +38,6 @@ using namespace LAMMPS_NS; using namespace MathConst; -static constexpr double BIG = 1.0e20; - /* ---------------------------------------------------------------------- */ ComputeRDF::ComputeRDF(LAMMPS *lmp, int narg, char **arg) : @@ -175,58 +173,19 @@ void ComputeRDF::init() if (!force->pair && !cutflag) error->all(FLERR,"Compute rdf requires a pair style or an explicit cutoff"); - // check if the pair style cutoff varies - double pairmaxcut, pairmincut; - if (force->pair) { - pairmaxcut = 0.0; - pairmincut = BIG; - for (int i = 1; i <= atom->ntypes; i++) - for (int j = i; j <= atom->ntypes; j++) { - const double cut = sqrt(force->pair->cutsq[i][j]); - pairmaxcut = MAX(pairmaxcut, cut); - pairmincut = MIN(pairmincut, cut); - } - } else { - pairmaxcut = pairmincut = cutoff_user; - } - if (cutflag) { mycutneigh = cutoff_user + skin; double cutghost; // as computed by Neighbor and Comm - double cutforce = 0.0; // largest pairwise cutoff plus skin - if (force->pair) cutforce = force->pair->cutforce + skin; - if (force->pair) - cutghost = MAX(force->pair->cutforce+skin,comm->cutghostuser); - else - cutghost = comm->cutghostuser; + if (force->pair) cutghost = MAX(force->pair->cutforce+skin,comm->cutghostuser); + else cutghost = comm->cutghostuser; if (mycutneigh > cutghost) error->all(FLERR,"Compute rdf cutoff plus skin {} exceeds ghost atom range {} - " "use comm_modify cutoff command to increase it", mycutneigh, cutghost); - // if the pair-wise cutoff varies for different pairs of types, the neighbor list code - // will still re-use the pairwise neighbor list if the *largest* cutoff is sufficient. - // this will lead to incorrect results and a larger user cutoff is required. - - if ((cutoff_user > pairmincut) && (cutoff_user <= pairmaxcut)) - error->all(FLERR,"Compute rdf cutoff {} must be larger than the maximum pair-wise cutoff {} " - "when the pair-wise cutoff varies", cutoff_user, pairmaxcut); - - if (force->pair && (cutoff_user < pairmaxcut)) { - if (comm->me == 0) - error->warning(FLERR,"Compute rdf cutoff {} is less than the pair-wise cutoff {} - " - "forcing a needless neighbor list build", cutoff_user, pairmaxcut); - } - delr = cutoff_user / nbin; - } else { - if ((pairmincut != pairmaxcut) && (comm->me == 0)) - error->warning(FLERR,"Pair-wise cutoff varies between {} and {}. Individual rdf functions " - "are only correct up to that cutoff - Use cutoff keyword to force a common " - "cutoff", pairmincut, pairmaxcut); - delr = force->pair->cutforce / nbin; - } + } delr = force->pair->cutforce / nbin; delrinv = 1.0/delr; From 82598ab3ca28db5545f3c29fd0b6ff8c38914380 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 16 Jan 2025 00:39:02 -0500 Subject: [PATCH 13/41] display error messages with fixed width font --- tools/lammps-gui/lammpsgui.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/lammps-gui/lammpsgui.cpp b/tools/lammps-gui/lammpsgui.cpp index 59e8017fc0..17d8e71206 100644 --- a/tools/lammps-gui/lammpsgui.cpp +++ b/tools/lammps-gui/lammpsgui.cpp @@ -1161,7 +1161,7 @@ void LammpsGui::run_done() status->setText("Failed."); ui->textEdit->setHighlight(nline, true); QMessageBox::critical(this, "LAMMPS-GUI Error", - QString("Error running LAMMPS:\n\n") + errorbuf); + QString("

Error running LAMMPS:\n\n

") + errorbuf + "

"); } ui->textEdit->setCursor(nline); ui->textEdit->setFileList(); From f2731166813920b9c390150f1e9895d86c80ff36 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 16 Jan 2025 10:18:16 -0500 Subject: [PATCH 14/41] prevent the neighbor list re-ordering from getting stuck --- src/neighbor.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/neighbor.cpp b/src/neighbor.cpp index 090fee9e5e..7daeeb4af4 100644 --- a/src/neighbor.cpp +++ b/src/neighbor.cpp @@ -1100,8 +1100,10 @@ int Neighbor::init_pair() NeighList *ptr; + // use counter to avoid getting stuck int done = 0; - while (!done) { + int count = 0; + while (!done && (count < 100)) { done = 1; for (i = 0; i < npair_perpetual; i++) { for (k = 0; k < 3; k++) { @@ -1110,8 +1112,9 @@ int Neighbor::init_pair() if (k == 1) ptr = lists[plist[i]]->listskip; if (k == 2) ptr = lists[plist[i]]->listfull; if (ptr == nullptr) continue; - for (m = 0; m < nrequest; m++) + for (m = 0; m < nrequest; m++) { if (ptr == lists[m]) break; + } for (j = 0; j < npair_perpetual; j++) if (m == plist[j]) break; if (j < i) continue; @@ -1123,7 +1126,11 @@ int Neighbor::init_pair() } if (!done) break; } + ++count; } + if (count == 100) + error->all(FLERR, "Failed to reorder neighbor lists to satisfy constraints - " + "Contact the LAMMPS developers for assistance"); // debug output From 6c16b1de744b657a04c98f9ae9bb5f82ca42614d Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 16 Jan 2025 10:26:48 -0500 Subject: [PATCH 15/41] must not set to unique if request is for skip list. only check for smallest pair cutoff. --- src/neighbor.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/neighbor.cpp b/src/neighbor.cpp index 7daeeb4af4..99f0e79efb 100644 --- a/src/neighbor.cpp +++ b/src/neighbor.cpp @@ -1193,9 +1193,8 @@ void Neighbor::morph_unique() for (int i = 0; i < nrequest; i++) { irq = requests[i]; - // if cut flag set by requestor and cutoff is different than default, - // or pair-wise cutoff is different for different pairs of atoms types - // set unique flag, otherwise unset cut flag + // if cut flag set by requestor and cutoff is larger than minimum for default, + // and the list is not a skip list, set unique flag; otherwise unset cut flag // this forces Pair,Stencil,Bin styles to be instantiated separately // also add skin to cutoff of perpetual lists @@ -1203,7 +1202,7 @@ void Neighbor::morph_unique() if (!irq->occasional) irq->cutoff += skin; - if ((irq->cutoff != cutneighmax) || (cutneighmin != cutneighmax)) { + if ((irq->cutoff > cutneighmin) && !irq->skip) { irq->unique = 1; } else { irq->cut = 0; From da5a12fcd11fdd9ced5de85e9ff9e9eb51da5a3d Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 16 Jan 2025 11:39:53 -0500 Subject: [PATCH 16/41] increase visibility of highlighting the failed argument --- src/utils.cpp | 7 ++++--- src/utils.h | 8 ++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/utils.cpp b/src/utils.cpp index 000baf88d0..bdb11ec9ab 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -132,7 +132,7 @@ void utils::missing_cmd_args(const std::string &file, int line, const std::strin if (error) error->all(file, line, "Illegal {} command: missing argument(s)", cmd); } -std::string utils::point_to_error(Input *input, int failedarg) +std::string utils::point_to_error(Input *input, int failed) { if (input) { std::string cmdline = "Preprocessed: "; @@ -142,14 +142,15 @@ std::string utils::point_to_error(Input *input, int failedarg) // assemble pre-processed command line and update error indicator position, if needed. for (int i = 0; i < input->narg; ++i) { - if (i == failedarg) indicator = cmdline.size(); + if (i == failed) indicator = cmdline.size(); cmdline += input->arg[i]; cmdline += ' '; } // construct and append error indicator line cmdline += '\n'; cmdline += std::string(indicator, ' '); - cmdline += "^\n"; + cmdline += std::string(strlen(input->arg[failed]), '^'); + cmdline += '\n'; return cmdline; } else return std::string("(Failed command line text not available)"); } diff --git a/src/utils.h b/src/utils.h index 78e59ab2c8..63f0643616 100644 --- a/src/utils.h +++ b/src/utils.h @@ -64,10 +64,10 @@ namespace utils { * * This function is a helper function for error messages. It creates * - * \param input pointer to the Input class instance (for access to last command args) - * \param failedarg index of the faulty argument (-1 to point to the command itself) - * \return string with two lines: the pre-processed command and a '^' pointing to the faulty argument */ - std::string point_to_error(Input *input, int failedarg); + * \param input pointer to the Input class instance (for access to last command args) + * \param faile index of the faulty argument (-1 to point to the command itself) + * \return string with two lines: the pre-processed command and a '^' pointing to the faulty argument */ + std::string point_to_error(Input *input, int failed); /*! Internal function handling the argument list for logmesg(). */ From fa54fd109795cbb16a4f1f5104aa7ff176e27ad6 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 16 Jan 2025 21:11:27 -0500 Subject: [PATCH 17/41] add option to suppress printing the last command --- src/error.cpp | 28 +++++++++++++--------------- src/error.h | 1 + src/fix_print.cpp | 11 +++++++---- src/utils.cpp | 17 +++++++++++++++-- src/variable.cpp | 8 ++++---- 5 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/error.cpp b/src/error.cpp index 1d656c1c45..d03e5453e9 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -118,36 +118,30 @@ void Error::all(const std::string &file, int line, int failed, const std::string { MPI_Barrier(world); - int me; std::string lastcmd = "(unknown)"; + std::string mesg = "ERROR: " + str + fmt::format(" ({}:{})\n", truncpath(file), line); - MPI_Comm_rank(world,&me); + if (input && input->line) lastcmd = input->line; - if (me == 0) { - std::string mesg = "ERROR: " + str; - - if (input && input->line) lastcmd = input->line; + if (failed > NOLASTLINE) { try { - mesg += fmt::format(" ({}:{})\nLast command: {}\n", truncpath(file),line,lastcmd); + mesg += fmt::format("Last input line: {}\n", lastcmd); if (failed > NOPOINTER) mesg += utils::point_to_error(input, failed); } catch (fmt::format_error &) { ; // do nothing } - utils::logmesg(lmp,mesg); } + if (comm->me == 0) utils::logmesg(lmp,mesg); // allow commands if an exception was caught in a run // update may be a null pointer when catching command-line errors if (update) update->whichflag = 0; - std::string msg = fmt::format("ERROR: {} ({}:{})\n", str, truncpath(file), line); - if (failed > NOPOINTER) msg += utils::point_to_error(input, failed); - if (universe->nworlds > 1) - throw LAMMPSAbortException(msg, universe->uworld); + throw LAMMPSAbortException(mesg, universe->uworld); else - throw LAMMPSException(msg); + throw LAMMPSException(mesg); } /* ---------------------------------------------------------------------- @@ -166,8 +160,12 @@ void Error::one(const std::string &file, int line, int failed, const std::string if (input && input->line) lastcmd = input->line; std::string mesg; try { - mesg = fmt::format("ERROR on proc {}: {} ({}:{})\nLast command: {}\n", - me,str,truncpath(file),line,lastcmd); + if (failed == NOLASTLINE) { + mesg = fmt::format("ERROR on proc {}: {} ({}:{})\n", me, str, truncpath(file), line); + } else { + mesg = fmt::format("ERROR on proc {}: {} ({}:{})\nLast input line: {}\n", + me, str, truncpath(file), line, lastcmd); + } if (failed > NOPOINTER) mesg += utils::point_to_error(input, failed); } catch (fmt::format_error &) { ; // do nothing diff --git a/src/error.h b/src/error.h index cf066338a4..0c446667e0 100644 --- a/src/error.h +++ b/src/error.h @@ -28,6 +28,7 @@ class Error : protected Pointers { void universe_warn(const std::string &, int, const std::string &); static constexpr int NOPOINTER = -2; + static constexpr int NOLASTLINE = -3; // regular error calls diff --git a/src/fix_print.cpp b/src/fix_print.cpp index ccef03c3ae..8e0a4a1921 100644 --- a/src/fix_print.cpp +++ b/src/fix_print.cpp @@ -38,7 +38,7 @@ FixPrint::FixPrint(LAMMPS *lmp, int narg, char **arg) : nevery = 1; } else { nevery = utils::inumeric(FLERR, arg[3], false, lmp); - if (nevery <= 0) error->all(FLERR, "Illegal fix print nevery value {}; must be > 0", nevery); + if (nevery <= 0) error->all(FLERR, 3, "Illegal fix print nevery value {}; must be > 0", nevery); } text = utils::strdup(arg[4]); @@ -121,12 +121,15 @@ void FixPrint::init() if (var_print) { ivar_print = input->variable->find(var_print); if (ivar_print < 0) - error->all(FLERR, "Variable {} for fix print timestep does not exist", var_print); + error->all(FLERR, Error::NOLASTLINE, "Variable {} for fix print timestep does not exist", + var_print); if (!input->variable->equalstyle(ivar_print)) - error->all(FLERR, "Variable {} for fix print timestep is invalid style", var_print); + error->all(FLERR, Error::NOLASTLINE, "Variable {} for fix print timestep is invalid style", + var_print); next_print = static_cast(input->variable->compute_equal(ivar_print)); if (next_print <= update->ntimestep) - error->all(FLERR, "Fix print timestep variable {} returned a bad timestep: {}", var_print, + error->all(FLERR, Error::NOLASTLINE, + "Fix print timestep variable {} returned a bad timestep: {}", var_print, next_print); } else { if (update->ntimestep % nevery) diff --git a/src/utils.cpp b/src/utils.cpp index bdb11ec9ab..4cb87d0131 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -135,15 +135,28 @@ void utils::missing_cmd_args(const std::string &file, int line, const std::strin std::string utils::point_to_error(Input *input, int failed) { if (input) { - std::string cmdline = "Preprocessed: "; + std::string cmdline = "--> parsed line: "; int indicator = cmdline.size(); // error indicator points to command by default cmdline += input->command; cmdline += ' '; // assemble pre-processed command line and update error indicator position, if needed. for (int i = 0; i < input->narg; ++i) { + std::string inputarg = input->arg[i]; if (i == failed) indicator = cmdline.size(); - cmdline += input->arg[i]; + + // argument contains whitespace. add quotes. check which type of quotes, too + if (inputarg.find_first_of(" \t\n") != std::string::npos) { + if (inputarg.find_first_of('"') != std::string::npos) { + cmdline += "'"; + cmdline += inputarg; + cmdline += "'"; + } else { + cmdline += '"'; + cmdline += inputarg; + cmdline += '"'; + } + } else cmdline += inputarg; cmdline += ' '; } // construct and append error indicator line diff --git a/src/variable.cpp b/src/variable.cpp index 031709166b..d20a134d0d 100644 --- a/src/variable.cpp +++ b/src/variable.cpp @@ -5227,14 +5227,14 @@ void Variable::print_var_error(const std::string &srcfile, const int lineno, if ((ivar >= 0) && (ivar < nvar)) { std::string msg = fmt::format("Variable {}: ",names[ivar]) + errmsg; if (global) - error->all(srcfile,lineno,msg); + error->all(srcfile, lineno, Error::NOLASTLINE, msg); else - error->one(srcfile,lineno,msg); + error->one(srcfile, lineno, Error::NOLASTLINE, msg); } else { if (global) - error->all(srcfile,lineno,errmsg); + error->all(srcfile,lineno, Error::NOLASTLINE, errmsg); else - error->one(srcfile,lineno,errmsg); + error->one(srcfile,lineno, Error::NOLASTLINE, errmsg); } } From 1636a1105474fc8476f4993f8f4f40c2070dd2db Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 16 Jan 2025 21:22:41 -0500 Subject: [PATCH 18/41] fix minor issues --- src/BROWNIAN/fix_brownian_base.cpp | 6 +++--- src/REAXFF/fix_reaxff_species.cpp | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/BROWNIAN/fix_brownian_base.cpp b/src/BROWNIAN/fix_brownian_base.cpp index 7c5f5deb8d..6b00ec2985 100644 --- a/src/BROWNIAN/fix_brownian_base.cpp +++ b/src/BROWNIAN/fix_brownian_base.cpp @@ -59,7 +59,7 @@ FixBrownianBase::FixBrownianBase(LAMMPS *lmp, int narg, char **arg) : int iarg = 5; while (iarg < narg) { if (strcmp(arg[iarg], "rng") == 0) { - if (narg == iarg + 1) utils::missing_cmd_args(FLERR, "fix brownian rng", error); + if (narg < iarg + 1) utils::missing_cmd_args(FLERR, "fix brownian rng", error); if (strcmp(arg[iarg + 1], "uniform") == 0) { noise_flag = 1; } else if (strcmp(arg[iarg + 1], "gaussian") == 0) { @@ -72,7 +72,7 @@ FixBrownianBase::FixBrownianBase(LAMMPS *lmp, int narg, char **arg) : } iarg = iarg + 2; } else if (strcmp(arg[iarg], "dipole") == 0) { - if (narg == iarg + 3) utils::missing_cmd_args(FLERR, "fix brownian dipole", error); + if (narg < iarg + 3) utils::missing_cmd_args(FLERR, "fix brownian dipole", error); dipole_flag = 1; delete[] dipole_body; @@ -84,7 +84,7 @@ FixBrownianBase::FixBrownianBase(LAMMPS *lmp, int narg, char **arg) : iarg = iarg + 4; } else if (strcmp(arg[iarg], "gamma_t_eigen") == 0) { - if (narg == iarg + 3) utils::missing_cmd_args(FLERR, "fix brownian gamma_t_eigen", error); + if (narg < iarg + 3) utils::missing_cmd_args(FLERR, "fix brownian gamma_t_eigen", error); gamma_t_eigen_flag = 1; delete[] gamma_t_inv; diff --git a/src/REAXFF/fix_reaxff_species.cpp b/src/REAXFF/fix_reaxff_species.cpp index c6da55fd25..4ef7799c24 100644 --- a/src/REAXFF/fix_reaxff_species.cpp +++ b/src/REAXFF/fix_reaxff_species.cpp @@ -204,6 +204,7 @@ FixReaxFFSpecies::FixReaxFFSpecies(LAMMPS *lmp, int narg, char **arg) : delete[] filedel; filedel = utils::strdup(arg[iarg + 1]); if (comm->me == 0) { + if (fdel) fclose(fdel); fdel = fopen(filedel, "w"); if (!fdel) error->one(FLERR, "Cannot open fix reaxff/species delete file {}: {}", filedel, From cf7695e99f18552fd5891d0128cdc8b0655b9d97 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 16 Jan 2025 21:43:36 -0500 Subject: [PATCH 19/41] add option to restart the LAMMPS instance to "Run" menu --- tools/lammps-gui/icons/system-restart.png | Bin 0 -> 4072 bytes tools/lammps-gui/lammpsgui.cpp | 1 + tools/lammps-gui/lammpsgui.h | 2 ++ tools/lammps-gui/lammpsgui.qrc | 1 + tools/lammps-gui/lammpsgui.ui | 11 ++++++++++- 5 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 tools/lammps-gui/icons/system-restart.png diff --git a/tools/lammps-gui/icons/system-restart.png b/tools/lammps-gui/icons/system-restart.png new file mode 100644 index 0000000000000000000000000000000000000000..62667c4390c78c4c5749a45d95750672058bd322 GIT binary patch literal 4072 zcmZ{ncTf}BpT(09LXZ$bk^Ug`o`fbb6ho5$F(A@=?=^HO(gi^w0s@L4T|tV{6p&t& zq9TG(6p;>6geXNomiONNW@l$-@67j}nLFn*=Y0RXX~rZy<}vJ%>|Jmwv#bK7xw%by_D``HCwj$bR?7k#O)C&aaI{VlA`@g$!rApB}u75 zfOU0Yi!dovQ{Ifb6=jPid146^F@ zIBqdV_I*+Rxw)}0uM$4K>(6j9@nr{ z)9|pF^uYi0ZCFkq#0NHgx6K;r_PhQ$Dq<*htfY?R^M+pmOYs4^=wJYT4$iVIf4Lj` z9Ez+^lS&uM528u`*}Z@BXEMX&mrTj-lE)gouu~ASvDv4wMX&-WgHQ}IhR_tI`-SfvZ%_hhs=Mrr(4VG zGKlSv^^shiasWRPA`B6(hFwob`%%67-0FQt@LZ7m z@pD01&DjrAW4)g77`T&GqESx4Es#8-z4Y4R z^0TzchbYp)vJRVyWdi1QKeW6OG$J4=2t|hzmQS2@58w`}*1T=QqRJFuMm~96^IjU%$f{)U|n1Z^YCS-0y?_0j}^HvUT!iU==N?Q5S6KZ(W%Wk@lE0g z^Bq~^W*A#|Mps_snOiYjy&CH5H>*7_%a++?O@sO}wSTE2y6y%|+}vUI9=NFh7z7&9gSp0dy-u8{<|k1 zXo9`*HaV@Z9N(lrebU8m^i9obeot)Apj-OAu}VOokyoXc^xSBqERWFJtKx<_94iyx z&u5Mq@W0l!?+ff35|(qhbqmbAouGeNXwL~hv}1Z@j;Yq$vT%S*5nqOyZ}veNA)SFZ zLx8Lklh3?Sz9zuc`24H|$Wim@W7j3zx9*zZ#(~ItLHumGb{>10UvVqe3{xQw?HGSK zBzy|)A!;o|udEhERGQO!j5>9urlyLO61}}$6?kSP-(HEwLq)xN3UDoDMJlt#q}*r1 z5!u^1H)gdjE`@-9fBwp1mkG$klf~aVK=oR>e}LfxY0#xhh0YfrwFt9Tn@CmbC$3*_ zurabTaDe0AS764dRQTbp%tu{`tV~J1j@!w|boR+2(!wUGT-WV@fUv@NJDJ>YRnlr+ z#9Fd49D1t2Hs}tdsN&oy0@^IDu$UXPA0|X?$CX3j7ytz*Qgw~Vk2m@ejZ$6jT&!Ta zir7b7MI_B0u33AoNTY_>8+Nh^LF3?-c|@~u;*Dq^Jgl!m*bD_@f^X9s#Hn*-iVd?s;FwDA0&MQL^y@SJ1Sk^%23}0`af!h{m?Af`24qBxI zWJnM0mvbqtOKouc8`IZ;;GXQ+w%Mz*2eZ#-pLzQ)Ki~Jq&yeX9R$;3t9N9z$F&5w* z`a89^V&T=Cb^{qc0>~FME?Cz|a0-jBHf*VZUw7~+H)SfHkb~e(i@TG_VtL6U6S&1J z;5<9)MD_}#qn`;Cg1(QLjw676E0)tVykvfIudJ4}Rup)d^TQa3T)qsxOulkFHVbJg z#TPpclz8uaO_`n`E%l9YgmopgZZRixFkG(n(_SXigG#2l900*Pu*XaEE>PF@8F50@ z6WIng~6V*Dq8WnnHWE(d@iYCjzWC`f_UzUHZq($pUiriVv3NF7!1Fy*3WssQ;EPy~Xdrk?8k` zFl~k~>UoXQxUJ_l1E6+hx5D^Jm9wzA618Sp{lG?JB<=N8Y2})4!8%pZO#3zbpqmIecxBjl#Mz_f~he!AWVM2e@^^twCZr zru;Qfo6nJenKz?y&7{SQjil}`$lHPSrlmTz0hE5p70UJ{v3aWNL)EjN!kz|yWu1Yj zPiI|sT}r7>^FXyHMa}&}km94w&F)ZM`!*aQPRedlC#;9E8`v9u#$xBiSEzt5Crk$d zp=fKCU$vMh<#P9%*FKEQ`u&!^bI-h8{*R{>B=;wUH&sQe6kZgSvD}xaz$9ik{OQ1X z=2qx>{_XO$71gX0C^tyLdX5oD>LZq_v}%7n%ql$dgwfnW;!_8VOjF($^dD^PmYIrn zxZh3>B{o!PZg4$dcxUMEG~P{m?xCXvIBs7k(o@+9X<~G~i71AL5e*-4Cqwi5?l=5V zqwecco7f=X*TQ~i3-+bUu>N)LNwoog#iC0=b)nvOp(*gA)gSd2JYran@}YqDO{veP z1zir;-Mv#Z0A1`WKnaDrOm`~s+OGEHy2{CRuq@3$%_q}i3a#o}|NbCSOmsPsm4^uz;;oQQQvfx}6Y*Jt$A2d0K&(ds*cqBi-#rA>7$@-dY;) zrC?GBgDa?c3AL%3>zl_ZCmZL1H)c@M>Yx{hN^yvxIhhZKSz%GYIDMx6b&Hd`C-!9c zm1A{Be&9U9yOv){ut~LI#yGU63C{qz(B{{S+NhSZ6Zy@ zg^f{MT-ZweBs_614^-JVLrUq~i3$2r&oVNGMTSSGZ1o4DphP8p-9eOle5O;JmYYO1K2+L>-#s{_}bquHtAy{m$T=LAmsG{{8-OH z1}k@D;FE8*?fI&k?U!eiqcoJCeTve(WOz|!&cMx-gmhFYNsdL8-iq481a4)|^;FHw zZMoT4VufraepPf%K0v+d2$0JTk3A+&k&1Pd#l>v4GyEUrEa5HP2kg`gU$k$rzdPBj zrvAv5Q={g&vzYU&e$@pJ+svOGJTmhL-l#T@oGhL2)PJwonQ@0*(!YYkaUa^ze4VBE z3+kL-bf!eR11vm-lV5@>-ffTy{sm0mKf20^D(pF5tLUlzLjSej!!lH^sFRh!w<05q zl1^n+UgsBe=QmTRmQAYObe;Xoph&v@;?4S#H&7yBXXK)+fRjnD(5598;2(D{=#@JY zYEk?8R9~Aa|&^jb3MHP3K-0JIeB?G1+1yO3Qiu2 zQ^3k$RB#xKJmsnEe}KQ8xPPfrKprE9kw0ZjF<6`u7N_{%Kzhqw?EeD%0$jb^BmaNk z_jjY-Q$YFeN<=RgH~(NafM-ZZAP$ZGC(A3$%_YFkFTg(-6+ret`~GVQMicdniD(6s zJX*D8B2f|8Y2&nGpX|%Pq`}?BwejfI97U+VLL}T|vpVIYj{ax+I-CEysla0wW4B Ap8x;= literal 0 HcmV?d00001 diff --git a/tools/lammps-gui/lammpsgui.cpp b/tools/lammps-gui/lammpsgui.cpp index 17d8e71206..602ae58900 100644 --- a/tools/lammps-gui/lammpsgui.cpp +++ b/tools/lammps-gui/lammpsgui.cpp @@ -212,6 +212,7 @@ LammpsGui::LammpsGui(QWidget *parent, const QString &filename) : connect(ui->actionRun_Buffer, &QAction::triggered, this, &LammpsGui::run_buffer); connect(ui->actionRun_File, &QAction::triggered, this, &LammpsGui::run_file); connect(ui->actionStop_LAMMPS, &QAction::triggered, this, &LammpsGui::stop_run); + connect(ui->actionRestart_LAMMPS, &QAction::triggered, this, &LammpsGui::restart_lammps); connect(ui->actionSet_Variables, &QAction::triggered, this, &LammpsGui::edit_variables); connect(ui->actionImage, &QAction::triggered, this, &LammpsGui::render_image); connect(ui->actionLAMMPS_Tutorial, &QAction::triggered, this, &LammpsGui::tutorial_web); diff --git a/tools/lammps-gui/lammpsgui.h b/tools/lammps-gui/lammpsgui.h index cb61f368a5..38fe00607e 100644 --- a/tools/lammps-gui/lammpsgui.h +++ b/tools/lammps-gui/lammpsgui.h @@ -111,6 +111,7 @@ private slots: void findandreplace(); void run_buffer() { do_run(true); } void run_file() { do_run(false); } + void restart_lammps() { lammps.close(); }; void edit_variables(); void render_image(); @@ -183,6 +184,7 @@ class TutorialWizard : public QWizard { public: TutorialWizard(int ntutorial, QWidget *parent = nullptr); void accept() override; + private: int _ntutorial; }; diff --git a/tools/lammps-gui/lammpsgui.qrc b/tools/lammps-gui/lammpsgui.qrc index 3f9697392b..51e091a11f 100644 --- a/tools/lammps-gui/lammpsgui.qrc +++ b/tools/lammps-gui/lammpsgui.qrc @@ -67,6 +67,7 @@ icons/search.png icons/system-box.png icons/system-help.png + icons/system-restart.png icons/system-run.png icons/trash.png icons/tutorial-logo.png diff --git a/tools/lammps-gui/lammpsgui.ui b/tools/lammps-gui/lammpsgui.ui index c6dbd6a507..ab922033bc 100644 --- a/tools/lammps-gui/lammpsgui.ui +++ b/tools/lammps-gui/lammpsgui.ui @@ -74,6 +74,7 @@ + @@ -270,7 +271,7 @@ - &Run LAMMPS from File + Run LAMMPS from &File Ctrl+Shift+Return @@ -287,6 +288,14 @@ Ctrl+/ + + + + + + Restart &LAMMPS + + From 3b815c1bbe77fa5843c82e354f3e948e88c6b343 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 16 Jan 2025 22:02:11 -0500 Subject: [PATCH 20/41] remove dead code --- src/LEPTON/fix_efield_lepton.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/LEPTON/fix_efield_lepton.cpp b/src/LEPTON/fix_efield_lepton.cpp index a055c2b15a..3a5c286476 100644 --- a/src/LEPTON/fix_efield_lepton.cpp +++ b/src/LEPTON/fix_efield_lepton.cpp @@ -228,7 +228,6 @@ void FixEfieldLepton::post_force(int vflag) double ex, ey, ez; double fx, fy, fz; double v[6], unwrap[3], dstep[3]; - double xf, yf, zf, xb, yb, zb; double exf, eyf, ezf, exb, eyb, ezb; double mu_norm, h_mu; From 1e179b2432a329546237524711d7140a48013e21 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 16 Jan 2025 23:56:34 -0500 Subject: [PATCH 21/41] address warnings reported by coverity scan --- src/EXTRA-PAIR/pair_dispersion_d3.cpp | 3 +- src/LEPTON/fix_efield_lepton.cpp | 48 +++++++++++++-------------- src/MEAM/meam_funcs.cpp | 1 + 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/EXTRA-PAIR/pair_dispersion_d3.cpp b/src/EXTRA-PAIR/pair_dispersion_d3.cpp index 6e2b3c1337..1c2c075b35 100644 --- a/src/EXTRA-PAIR/pair_dispersion_d3.cpp +++ b/src/EXTRA-PAIR/pair_dispersion_d3.cpp @@ -481,8 +481,6 @@ void PairDispersionD3::compute(int eflag, int vflag) int jnum = numneigh[i]; int *jlist = firstneigh[i]; - // fprintf(stderr, "> i, type[i], CN[i], C6[i,i] : %d, %d, %f, %f\n", atom->tag[i], type[i], cn[i], get_dC6(type[i],type[i],cn[i],cn[i])[0]/(autoev*pow(autoang,6))); - for (int jj = 0; jj < jnum; jj++) { int j = jlist[jj]; @@ -513,6 +511,7 @@ void PairDispersionD3::compute(int eflag, int vflag) double t6, t8, damp6, damp8, e6, e8; double tmp6, tmp8, fpair1, fpair2, fpair; + t6 = t8 = e6 = e8 = evdwl = fpair = fpair1 = fpair2 = 0.0; switch (dampingCode) { case 1: { // zero diff --git a/src/LEPTON/fix_efield_lepton.cpp b/src/LEPTON/fix_efield_lepton.cpp index 3a5c286476..d3b82b40c5 100644 --- a/src/LEPTON/fix_efield_lepton.cpp +++ b/src/LEPTON/fix_efield_lepton.cpp @@ -45,7 +45,8 @@ FixEfieldLepton::FixEfieldLepton(LAMMPS *lmp, int narg, char **arg) : Fix(lmp, narg, arg), idregion(nullptr), region(nullptr) { if (domain->xperiodic || domain->yperiodic || domain->zperiodic) { - error->warning(FLERR, "Fix {} uses unwrapped coordinates", style); + if (comm->me == 0) + error->warning(FLERR, "Fix {} uses unwrapped coordinates", style); } if (narg < 4) utils::missing_cmd_args(FLERR, std::string("fix ") + style, error); @@ -65,12 +66,13 @@ FixEfieldLepton::FixEfieldLepton(LAMMPS *lmp, int narg, char **arg) : utils::missing_cmd_args(FLERR, std::string("fix ") + style + " region", error); region = domain->get_region_by_id(arg[iarg + 1]); if (!region) error->all(FLERR, "Region {} for fix {} does not exist", arg[iarg + 1], style); + delete[] idregion; idregion = utils::strdup(arg[iarg + 1]); iarg += 2; } else if (strcmp(arg[iarg], "step") == 0) { if (iarg + 2 > narg) utils::missing_cmd_args(FLERR, std::string("fix ") + style + "step", error); - h = utils::numeric(FLERR, arg[iarg+1], false, lmp); + h = utils::numeric(FLERR, arg[iarg + 1], false, lmp); iarg += 2; } else { error->all(FLERR, "Unknown keyword for fix {} command: {}", style, arg[iarg]); @@ -126,7 +128,8 @@ void FixEfieldLepton::init() } if (utils::strmatch(update->integrate_style, "^respa")) { - ilevel_respa = (dynamic_cast(update->integrate))->nlevels - 1; + auto respa = dynamic_cast(update->integrate); + if (respa) ilevel_respa = respa->nlevels - 1; if (respa_level >= 0) ilevel_respa = MIN(respa_level, ilevel_respa); } @@ -134,7 +137,8 @@ void FixEfieldLepton::init() char *unit_style = update->unit_style; qe2f = force->qe2f; mue2e = qe2f; - if (strcmp(unit_style, "electron") == 0 || strcmp(unit_style, "micro") == 0 || strcmp(unit_style, "nano") == 0) { + if (strcmp(unit_style, "electron") == 0 || strcmp(unit_style, "micro") == 0 || + strcmp(unit_style, "nano") == 0) { error->all(FLERR, "Fix {} does not support {} units", style, unit_style); } } @@ -145,9 +149,11 @@ void FixEfieldLepton::setup(int vflag) { if (utils::strmatch(update->integrate_style, "^respa")) { auto respa = dynamic_cast(update->integrate); - respa->copy_flevel_f(ilevel_respa); - post_force_respa(vflag, ilevel_respa, 0); - respa->copy_f_flevel(ilevel_respa); + if (respa) { + respa->copy_flevel_f(ilevel_respa); + post_force_respa(vflag, ilevel_respa, 0); + respa->copy_f_flevel(ilevel_respa); + } } else { post_force(vflag); } @@ -179,14 +185,14 @@ void FixEfieldLepton::post_force(int vflag) auto dphi_x = parsed.differentiate("x").createCompiledExpression(); auto dphi_y = parsed.differentiate("y").createCompiledExpression(); auto dphi_z = parsed.differentiate("z").createCompiledExpression(); - std::array dphis = {&dphi_x, &dphi_y, &dphi_z}; + std::array dphis = {&dphi_x, &dphi_y, &dphi_z}; // array of vectors of ptrs to Lepton variable references std::array, 3> var_ref_ptrs{}; // fill ptr-vectors with Lepton refs as needed - const char* DIM_NAMES[] = {"x", "y", "z"}; - if (atom->q_flag){ + const char *DIM_NAMES[] = {"x", "y", "z"}; + if (atom->q_flag) { phi = parsed.createCompiledExpression(); for (size_t d = 0; d < 3; d++) { try { @@ -205,13 +211,13 @@ void FixEfieldLepton::post_force(int vflag) double *ptr = &((*dphis[j]).getVariableReference(DIM_NAMES[d])); var_ref_ptrs[d].push_back(ptr); e_uniform = false; - } - catch (Lepton::Exception &) { + } catch (Lepton::Exception &) { // do nothing } } if (!e_uniform && atom->mu_flag && h < 0) { - error->all(FLERR, "Fix {} requires keyword `step' for dipoles in a non-uniform electric field", style); + error->all(FLERR, "Fix {} requires keyword `step' for dipoles in a non-uniform electric field", + style); } // virial setup @@ -243,9 +249,7 @@ void FixEfieldLepton::post_force(int vflag) // put unwrapped coords into Lepton variable refs for (size_t d = 0; d < 3; d++) { - for (auto & var_ref_ptr : var_ref_ptrs[d]) { - *var_ref_ptr = unwrap[d]; - } + for (auto &var_ref_ptr : var_ref_ptrs[d]) { *var_ref_ptr = unwrap[d]; } } // evaluate e-field, used by q and mu @@ -264,8 +268,8 @@ void FixEfieldLepton::post_force(int vflag) } if (atom->mu_flag) { - // dipoles - mu_norm = sqrt(mu[i][0]*mu[i][0] + mu[i][1]*mu[i][1] + mu[i][2]*mu[i][2]); + // dipoles + mu_norm = sqrt(mu[i][0] * mu[i][0] + mu[i][1] * mu[i][1] + mu[i][2] * mu[i][2]); if (mu_norm > EPSILON) { // torque = mu cross E t[i][0] += mue2e * (ez * mu[i][1] - ey * mu[i][2]); @@ -284,9 +288,7 @@ void FixEfieldLepton::post_force(int vflag) // one step forwards, two steps back ;) for (size_t d = 0; d < 3; d++) { - for (auto & var_ref_ptr : var_ref_ptrs[d]) { - *var_ref_ptr += dstep[d]; - } + for (auto &var_ref_ptr : var_ref_ptrs[d]) { *var_ref_ptr += dstep[d]; } } exf = -dphi_x.evaluate(); @@ -294,9 +296,7 @@ void FixEfieldLepton::post_force(int vflag) ezf = -dphi_z.evaluate(); for (size_t d = 0; d < 3; d++) { - for (auto & var_ref_ptr : var_ref_ptrs[d]) { - *var_ref_ptr -= 2*dstep[d]; - } + for (auto &var_ref_ptr : var_ref_ptrs[d]) { *var_ref_ptr -= 2 * dstep[d]; } } exb = -dphi_x.evaluate(); diff --git a/src/MEAM/meam_funcs.cpp b/src/MEAM/meam_funcs.cpp index b08d8380b3..e769802b38 100644 --- a/src/MEAM/meam_funcs.cpp +++ b/src/MEAM/meam_funcs.cpp @@ -44,6 +44,7 @@ double MEAM::G_gam(const double gamma, const int ibar, int &errorflag) const // e.g. gsmooth_factor is 99, {: // gsmooth_switchpoint = -0.99 // G = 0.01*(-0.99/gamma)**99 + if (gamma == 0.0) return 0.0; // avoid division by zero. For gamma = 0.0 => G = 1 / inf double G = 1 / (gsmooth_factor + 1) * pow((gsmooth_switchpoint / gamma), gsmooth_factor); return sqrt(G); } else { From a4cc00041cc06bee939cf155096a53092649fdea Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 17 Jan 2025 05:13:25 -0500 Subject: [PATCH 22/41] add notes about errors and warnings and log output --- doc/src/Developer_notes.rst | 139 ++++++++++++++++++++++++++++++++++-- doc/src/Developer_utils.rst | 3 + 2 files changed, 135 insertions(+), 7 deletions(-) diff --git a/doc/src/Developer_notes.rst b/doc/src/Developer_notes.rst index af26b4b913..a562ee26e4 100644 --- a/doc/src/Developer_notes.rst +++ b/doc/src/Developer_notes.rst @@ -7,13 +7,7 @@ typically document what a variable stores, what a small section of code does, or what a function does and its input/outputs. The topics on this page are intended to document code functionality at a higher level. -Available topics are: - -- `Reading and parsing of text and text files`_ -- `Requesting and accessing neighbor lists`_ -- `Choosing between a custom atom style, fix property/atom, and fix STORE/ATOM`_ -- `Fix contributions to instantaneous energy, virial, and cumulative energy`_ -- `KSpace PPPM FFT grids`_ +.. contents:: ---- @@ -218,6 +212,137 @@ command: neighbor->add_request(this, "delete_atoms", NeighConst::REQ_FULL); + +Errors, warnings, and informational messages +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +LAMMPS has specialized functionality to handle errors (which should +terminate LAMMPS), warning messages (which should indicate possible +problems *without* terminating LAMMPS), and informational text for +messages about the progress and chosen settings. We *strongly* +encourage using these facilities and to *stay away* from using +``printf()`` or ``fprintf()`` or ``std::cout`` or ``std::cerr`` and +calling ``MPI_Abort()`` or ``exit()`` directly. Warnings and +informational messages should be printed only on MPI rank 0 to avoid +flooding the output when running in parallel with many MPI processes. + +**Errors** + +When LAMMPS encounters an error, for example a syntax error in the +input, then a suitable error message should be printed giving a brief, +one line remark about the reason and then call either ``Error::all()`` +or ``Error::one()``. ``Error::all()`` must be called when the failing +code path is executed by *all* MPI processes and the error condition +will appear for *all* MPI processes the same. If desired, each MPI +process may set a flag to either 0 or 1 and then MPI_Allreduce() +searching for the maximum can be used to determine if there was an error +on *any* of the MPI processes and make this information available to +*all*. ``Error::one()`` in contrast needs to be called when only one or +a few MPI processes execute the code path or can have the error +condition. ``Error::all()`` is generally the preferred option. + +Calling these functions does not abort LAMMPS directly, but rather +throws either a ``LAMMPSException`` (from ``Error::all()``) or a +``LAMMPSAbortException`` (from ``Error::one()``). These exceptions are +caught by the LAMMPS ``main()`` program and then handled accordingly. +The reason for this approach is to support applications, especially +graphical applications like :ref:`LAMMPS-GUI `, that are +linked to the LAMMPS library and have a mechanism to avoid that an error +in LAMMPS terminates the application. By catching the exceptions, the +application can delete the failing LAMMPS class instance and create a +new one to try again. In a similar fashion, the :doc:`LAMMPS Python +module ` checks for this and then re-throws corresponding +Python exception, which in turn can be caught by the calling Python +code. + +There are multiple "signatures" that can be called: + +- ``Error::all(FLERR, "Error message")``: this will abort LAMMPS with + the error message "Error message", followed by the last line of input + that was read and processed before the error condition happened. + +- ``Error::all(FLERR, Error::NOLASTLINE, "Error message")``: this is the + same as before but without the last line of input. This is preferred + for errors that would happen *during* a :doc:`run ` or + :doc:`minimization `, since showing the "run" or "minimize" + command would be the last line, but is unrelated to the error. + +- ``Error::all(FLERR, idx, "Error message")``: this is for argument + parsing where "idx" is the index of the argument for a LAMMPS command + that is causing the failure (use -1 for the command itself). The + output will then not only be the last input line, but also the command + and arguments *after* they have been pre-processed and split into + individual arguments and a textual indicator pointing to the specific + word that failed. + +FLERR is a macro containing the filename and line where the Error class +is called and that information is appended to the error message. This +allows to quickly find the relevant source code causing the error. For +all three signatures, the single string "Error message" may be replaced +with a format string using '{}' placeholders and followed by a variable +number of arguments, one for each placeholder. This format string and +the arguments are then handed for formatting to the `{fmt} library +`_ (which is bundled with LAMMPS) and thus allow +processing similar to the "format()" functionality in Python. + +For complex errors, that can have multiple causes and which cannot be +explained in a single line, you can append to the error message, the +string created by :cpp:func:`utils::errorurl`, which then provides a +URL pointing to a paragraph of the :doc:`Errors_details` that +corresponds to the number provided. Example: + +.. code-block:: c++ + + error->all(FLERR, "Unknown identifier in data file: {}{}", keyword, utils::errorurl(1)); + +This will output something like this: + +.. parsed-literal:: + + ERROR: Unknown identifier in data file: Massess + For more information see https://docs.lammps.org/err0001 (src/read_data.cpp:1482) + Last input line: read_data data.peptide + +Where the URL points to the first paragraph with explanations on +the :doc:`Errors_details` page in the manual. + +**Warnings** + +To print warnings, the ``Errors::warning()`` function should be used. +It also requires the FLERR macros as first argument to easily identify +the location of the warning in the source code. Same as with the error +functions above, the function has two variants: one just taking a single +string as final argument and a second that uses the `{fmt} library +`_ to make it similar to, say, ``fprintf()``. One +motivation to use this function is that it will output warnings with +always the same capitalization of the leading "WARNING" string. A +second is that it has a built in rate limiter. After a given number (by +default 100), that can be set via the :doc:`thermo_modify command +` no more warnings are printed. Also, warnings are +written consistently to both screen and logfile or not, depending on the +settings for :ref:`screen ` or :doc:`logfile ` output. + +.. note:: + + Unlike ``Error::all()``, the warning function will produce output on + *every* MPI process, so it typically would be prefixed with an if + statement testing for ``comm->me == 0``, i.e. limiting output to MPI + rank 0. + +**Informational messages** + +Finally, for informational message LAMMPS has the +:cpp:func:`utils::logmesg() convenience function +`. It also uses the `{fmt} library +`_ to support using a format string followed by a +matching number of arguments. It will output the resulting formatted +text to both, the screen and the logfile and will honor the +corresponding settings about whether this output is active and to which +file it should be send. Same as for ``Error::warning()``, it would +produce output for every MPI process and thus should usually be called +only on MPI rank 0 to avoid flooding the output when running with many +parallel processes. + Choosing between a custom atom style, fix property/atom, and fix STORE/ATOM ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/doc/src/Developer_utils.rst b/doc/src/Developer_utils.rst index b7f62d5364..402ed680f7 100644 --- a/doc/src/Developer_utils.rst +++ b/doc/src/Developer_utils.rst @@ -238,6 +238,9 @@ Convenience functions .. doxygenfunction:: missing_cmd_args :project: progguide +.. doxygenfunction:: point_to_error + :project: progguide + .. doxygenfunction:: flush_buffers(LAMMPS *lmp) :project: progguide From 7c4649adbf941e63e76cf713da74fb94fbff6eec Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 17 Jan 2025 08:43:46 -0500 Subject: [PATCH 23/41] update TODO list and changelog --- tools/lammps-gui/TODO.md | 3 --- tools/lammps-gui/lammps-gui.appdata.xml | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/lammps-gui/TODO.md b/tools/lammps-gui/TODO.md index 17fa4c04ee..b978931f0c 100644 --- a/tools/lammps-gui/TODO.md +++ b/tools/lammps-gui/TODO.md @@ -2,7 +2,6 @@ LAMMPS-GUI TODO list: # Short term goals (v1.x) -- add a preferences option to override light/dark theme setting and add choice for theme - implement a timed "Auto-Save" feature that saves after some idle time. set timeout in Editor preferences. - add a "Filter data" checkbox to the "Charts" window to select whether data should be dropped. - add a "Charts tab" to the preferences with the following (default) settings: @@ -16,8 +15,6 @@ LAMMPS-GUI TODO list: colors to individual atom types. - Support color by property (e.g. scan computes or fixes with per-atom data), define colormaps etc. - Add a "Diameters" dialog where diamaters can by specified by atom type -- figure out how widgets can be resized to fraction of available screen size. -- figure out stacking order of frames and whether it can be more flexible - implement indenting regions for (nested) loops? - implement data file manager GUI with the following features: diff --git a/tools/lammps-gui/lammps-gui.appdata.xml b/tools/lammps-gui/lammps-gui.appdata.xml index c019ab1ce3..64d8892956 100644 --- a/tools/lammps-gui/lammps-gui.appdata.xml +++ b/tools/lammps-gui/lammps-gui.appdata.xml @@ -62,6 +62,7 @@ Make Tutorial wizards more compact Include download and compilation of WHAM software from Alan Grossfield Add dialog to run WHAM directly from LAMMPS-GUI + Add entry to Run menu to restart the LAMMPS instance Use mutex to avoid corruption of thermo data From 0cb64afc84a6dad8bc9bec03d2909112a3794517 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 17 Jan 2025 09:13:55 -0500 Subject: [PATCH 24/41] explain error message output for users --- doc/src/Run_output.rst | 48 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/doc/src/Run_output.rst b/doc/src/Run_output.rst index 2025bf5321..97dfac75d4 100644 --- a/doc/src/Run_output.rst +++ b/doc/src/Run_output.rst @@ -178,3 +178,51 @@ with and without the communication and a Gflop rate is computed. The 3d rate is with communication; the 1d rate is without (just the 1d FFTs). Thus you can estimate what fraction of your FFT time was spent in communication, roughly 75% in the example above. + +Error message output +==================== + +Depending on the error function arguments when it is called in the +source code, there will be one to four lines of error output. + +A single line +^^^^^^^^^^^^^ + +The line starts with "ERROR: ", followed by the error message and +information about the location in the source where the error function +was called in parenthesis on the right (here: line 131 of the file +src/fix_print.cpp). Example: + +.. parsed-literal:: + + ERROR: Fix print timestep variable nevery returned a bad timestep: 9900 (src/fix_print.cpp:131) + +Two lines +^^^^^^^^^ + +In addition to the single line output, also the last line of the input +will be repeated. If a command is spread over multiple lines in the +input using the continuation character '&', then the error will print +the entire concatenated line. However, there is no further processing +of that line. Example: + +.. parsed-literal:: + + ERROR: Unrecognized fix style 'printf' (src/modify.cpp:924) + Last input line: fix 0 all printf v_nevery 'Step: $(step) ${step}' + +Four lines +^^^^^^^^^^ + +In addition to the two line output, the last command is printed a second +time, but *after* all variables were substituted and the line separated +into "words" as they are passed internally to the called command. This line +is followed by a line with caret character markers '^' to indicate which +"word" in the parsed input is causing the failure. Example: + +.. parsed-literal:: + + ERROR: Illegal fix print nevery value -100; must be > 0 (src/fix_print.cpp:41) + Last input line: fix 0 all print ${nevery} 'Step: $(step) ${step}' + --> parsed line: fix 0 all print -100 "Step: $(step) ${step}" + ^^^^ From 5a45ef6994a50e1f68ba06fb04834da763629281 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 17 Jan 2025 09:13:55 -0500 Subject: [PATCH 25/41] explain error message output for users --- doc/src/Run_output.rst | 48 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/doc/src/Run_output.rst b/doc/src/Run_output.rst index 2025bf5321..d79882511a 100644 --- a/doc/src/Run_output.rst +++ b/doc/src/Run_output.rst @@ -178,3 +178,51 @@ with and without the communication and a Gflop rate is computed. The 3d rate is with communication; the 1d rate is without (just the 1d FFTs). Thus you can estimate what fraction of your FFT time was spent in communication, roughly 75% in the example above. + +Error message output +==================== + +Depending on the error function arguments when it is called in the +source code, there will be one to four lines of error output. + +A single line +^^^^^^^^^^^^^ + +The line starts with "ERROR: ", followed by the error message and +information about the location in the source where the error function +was called in parenthesis on the right (here: line 131 of the file +src/fix_print.cpp). Example: + +.. parsed-literal:: + + ERROR: Fix print timestep variable nevery returned a bad timestep: 9900 (src/fix_print.cpp:131) + +Two lines +^^^^^^^^^ + +In addition to the single line output, also the last line of the input +will be repeated. If a command is spread over multiple lines in the +input using the continuation character '&', then the error will print +the entire concatenated line. However, there is no further processing +of that line. Example: + +.. parsed-literal:: + + ERROR: Unrecognized fix style 'printf' (src/modify.cpp:924) + Last input line: fix 0 all printf v_nevery 'Step: $(step) ${step}' + +Four lines +^^^^^^^^^^ + +In addition to the two line output, the last command is printed a second +time, but *after* all variables were substituted and the line separated +into "words" as they are passed internally to the called command. This line +is followed by a line with caret character markers '^' to indicate which +"word" in the parsed input is causing the failure. Example: + +.. parsed-literal:: + + ERROR: Illegal fix print nevery value -100; must be > 0 (src/fix_print.cpp:41) + Last input line: fix 0 all print ${nevery} 'Step: $(step) ${step}' + --> parsed line: fix 0 all print -100 "Step: $(step) ${step}" + ^^^^ From b9dbfc6eb23f428f15221126464c1f7657c8efc5 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 17 Jan 2025 10:40:34 -0500 Subject: [PATCH 26/41] add support to flag failed arguments for calls to expand_args() --- src/fix_ave_time.cpp | 41 ++++++++++++++++++------------- src/fix_ave_time.h | 1 + src/utils.cpp | 57 ++++++++++++++++++++++++++++---------------- src/utils.h | 27 +++++++++++++-------- 4 files changed, 78 insertions(+), 48 deletions(-) diff --git a/src/fix_ave_time.cpp b/src/fix_ave_time.cpp index 72ff8ab6c1..8c7d902b86 100644 --- a/src/fix_ave_time.cpp +++ b/src/fix_ave_time.cpp @@ -68,7 +68,7 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : } else break; } if (nvalues == 0) - error->all(FLERR,"No values from computes, fixes, or variables used in fix ave/time command"); + error->all(FLERR, 6, "No values from computes, fixes, or variables used in fix ave/time command"); // parse optional keywords @@ -79,7 +79,8 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : int expand = 0; char **earg; - nvalues = utils::expand_args(FLERR,nvalues,&arg[6],mode,earg,lmp); + int *amap; + nvalues = utils::expand_args(FLERR,nvalues,&arg[6],mode,earg,lmp,&amap); key2col.clear(); if (earg != &arg[6]) expand = 1; @@ -97,9 +98,10 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : key2col[arg[i]] = i; if ((val.which == ArgInfo::NONE) || (val.which == ArgInfo::UNKNOWN) || (argi.get_dim() > 1)) - error->all(FLERR,"Invalid fix ave/time argument: {}", arg[i]); + error->all(FLERR, amap[i]+6,"Invalid fix ave/time argument: {}", arg[i]); val.argindex = argi.get_index1(); + val.iarg = amap[i] + 6; val.varlen = 0; val.offcol = 0; val.id = argi.get_name(); @@ -122,9 +124,9 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : // for fix inputs, check that fix frequency is acceptable // set variable_length if any compute is variable length - if (nevery <= 0) error->all(FLERR,"Illegal fix ave/time nevery value: {}", nevery); - if (nrepeat <= 0) error->all(FLERR,"Illegal fix ave/time nrepeat value: {}", nrepeat); - if (nfreq <= 0) error->all(FLERR,"Illegal fix ave/time nfreq value: {}", nfreq); + if (nevery <= 0) error->all(FLERR, 3, "Illegal fix ave/time nevery value: {}", nevery); + if (nrepeat <= 0) error->all(FLERR, 4, "Illegal fix ave/time nrepeat value: {}", nrepeat); + if (nfreq <= 0) error->all(FLERR, 5, "Illegal fix ave/time nfreq value: {}", nfreq); if (nfreq % nevery || nrepeat*nevery > nfreq) error->all(FLERR,"Inconsistent fix ave/time nevery/nrepeat/nfreq values"); if (ave != RUNNING && overwrite) @@ -134,25 +136,29 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : if ((val.which == ArgInfo::COMPUTE) && (mode == SCALAR)) { val.val.c = modify->get_compute_by_id(val.id); - if (!val.val.c) error->all(FLERR,"Compute ID {} for fix ave/time does not exist", val.id); + if (!val.val.c) + error->all(FLERR, val.iarg, "Compute ID {} for fix ave/time does not exist", val.id); if (val.argindex == 0 && (val.val.c->scalar_flag == 0)) - error->all(FLERR,"Fix ave/time compute {} does not calculate a scalar", val.id); + error->all(FLERR, val.iarg, "Fix ave/time compute {} does not calculate a scalar", val.id); if (val.argindex && (val.val.c->vector_flag == 0)) - error->all(FLERR,"Fix ave/time compute {} does not calculate a vector", val.id); + error->all(FLERR, val.iarg, "Fix ave/time compute {} does not calculate a vector", val.id); if (val.argindex && (val.argindex > val.val.c->size_vector) && (val.val.c->size_vector_variable == 0)) - error->all(FLERR, "Fix ave/time compute {} vector is accessed out-of-range", val.id); + error->all(FLERR, val.iarg, "Fix ave/time compute {} vector is accessed out-of-range", + val.id); if (val.argindex && val.val.c->size_vector_variable) val.varlen = 1; } else if ((val.which == ArgInfo::COMPUTE) && (mode == VECTOR)) { val.val.c = modify->get_compute_by_id(val.id); - if (!val.val.c) error->all(FLERR,"Compute ID {} for fix ave/time does not exist", val.id); + if (!val.val.c) + error->all(FLERR, val.iarg, "Compute ID {} for fix ave/time does not exist", val.id); if ((val.argindex == 0) && (val.val.c->vector_flag == 0)) - error->all(FLERR,"Fix ave/time compute {} does not calculate a vector", val.id); + error->all(FLERR, val.iarg, "Fix ave/time compute {} does not calculate a vector", val.id); if (val.argindex && (val.val.c->array_flag == 0)) - error->all(FLERR,"Fix ave/time compute {} does not calculate an array", val.id); + error->all(FLERR, val.iarg, "Fix ave/time compute {} does not calculate an array", val.id); if (val.argindex && (val.argindex > val.val.c->size_array_cols)) - error->all(FLERR,"Fix ave/time compute {} array is accessed out-of-range", val.id); + error->all(FLERR, val.iarg, "Fix ave/time compute {} array is accessed out-of-range", + val.id); if ((val.argindex == 0) && (val.val.c->size_vector_variable)) val.varlen = 1; if (val.argindex && (val.val.c->size_array_rows_variable)) val.varlen = 1; @@ -1042,7 +1048,7 @@ void FixAveTime::options(int iarg, int narg, char **arg) if (strcmp(arg[iarg],"file") == 0) fp = fopen(arg[iarg+1],"w"); else fp = fopen(arg[iarg+1],"a"); if (fp == nullptr) - error->one(FLERR,"Cannot open fix ave/time file {}: {}", + error->one(FLERR, iarg+1, "Cannot open fix ave/time file {}: {}", arg[iarg+1], utils::getsyserror()); } iarg += 2; @@ -1051,12 +1057,13 @@ void FixAveTime::options(int iarg, int narg, char **arg) if (strcmp(arg[iarg+1],"one") == 0) ave = ONE; else if (strcmp(arg[iarg+1],"running") == 0) ave = RUNNING; else if (strcmp(arg[iarg+1],"window") == 0) ave = WINDOW; - else error->all(FLERR,"Unknown fix ave/time ave keyword {}", arg[iarg+1]); + else error->all(FLERR, iarg+1, "Unknown fix ave/time ave keyword {}", arg[iarg+1]); if (ave == WINDOW) { if (iarg+3 > narg) utils::missing_cmd_args(FLERR, "fix ave/time ave window", error); nwindow = utils::inumeric(FLERR,arg[iarg+2],false,lmp); if (nwindow <= 0) - error->all(FLERR,"Illegal fix ave/time ave window argument {}; must be > 0", nwindow); + error->all(FLERR, iarg+2, "Illegal fix ave/time ave window argument {}; must be > 0", + nwindow); } iarg += 2; if (ave == WINDOW) iarg++; diff --git a/src/fix_ave_time.h b/src/fix_ave_time.h index 5b25a68ab5..5aedc4443d 100644 --- a/src/fix_ave_time.h +++ b/src/fix_ave_time.h @@ -43,6 +43,7 @@ class FixAveTime : public Fix { struct value_t { int which; // type of data: COMPUTE, FIX, VARIABLE int argindex; // 1-based index if data is vector, else 0 + int iarg; // argument index in original argument list int varlen; // 1 if value is from variable-length compute int offcol; std::string id; // compute/fix/variable ID diff --git a/src/utils.cpp b/src/utils.cpp index 4cb87d0131..90d165202a 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -136,7 +136,7 @@ std::string utils::point_to_error(Input *input, int failed) { if (input) { std::string cmdline = "--> parsed line: "; - int indicator = cmdline.size(); // error indicator points to command by default + int indicator = cmdline.size(); // error indicator points to command by default cmdline += input->command; cmdline += ' '; @@ -156,7 +156,8 @@ std::string utils::point_to_error(Input *input, int failed) cmdline += inputarg; cmdline += '"'; } - } else cmdline += inputarg; + } else + cmdline += inputarg; cmdline += ' '; } // construct and append error indicator line @@ -165,7 +166,8 @@ std::string utils::point_to_error(Input *input, int failed) cmdline += std::string(strlen(input->arg[failed]), '^'); cmdline += '\n'; return cmdline; - } else return std::string("(Failed command line text not available)"); + } else + return std::string("(Failed command line text not available)"); } /* specialization for the case of just a single string argument */ @@ -682,14 +684,14 @@ tagint utils::tnumeric(const char *file, int line, const char *str, bool do_abor // clang-format off template void utils::bounds(const char *file, int line, const std::string &str, - bigint nmin, bigint nmax, TYPE &nlo, TYPE &nhi, Error *error) + bigint nmin, bigint nmax, TYPE &nlo, TYPE &nhi, Error *error, int failed) { nlo = nhi = -1; // check for illegal characters size_t found = str.find_first_not_of("*-0123456789"); if (found != std::string::npos) { - if (error) error->all(file, line, "Invalid range string: {}", str); + if (error) error->all(file, line, failed, "Invalid range string: {}", str); return; } @@ -712,23 +714,23 @@ void utils::bounds(const char *file, int line, const std::string &str, if (error) { if ((nlo <= 0) || (nhi <= 0)) - error->all(file, line, "Invalid range string: {}", str); + error->all(file, line, failed, "Invalid range string: {}", str); if (nlo < nmin) - error->all(file, line, "Numeric index {} is out of bounds ({}-{})", nlo, nmin, nmax); + error->all(file, line, failed, "Numeric index {} is out of bounds ({}-{})", nlo, nmin, nmax); else if (nhi > nmax) - error->all(file, line, "Numeric index {} is out of bounds ({}-{})", nhi, nmin, nmax); + error->all(file, line, failed, "Numeric index {} is out of bounds ({}-{})", nhi, nmin, nmax); else if (nlo > nhi) - error->all(file, line, "Numeric index {} is out of bounds ({}-{})", nlo, nmin, nhi); + error->all(file, line, failed, "Numeric index {} is out of bounds ({}-{})", nlo, nmin, nhi); } } template void utils::bounds<>(const char *, int, const std::string &, - bigint, bigint, int &, int &, Error *); + bigint, bigint, int &, int &, Error *, int); template void utils::bounds<>(const char *, int, const std::string &, - bigint, bigint, long &, long &, Error *); + bigint, bigint, long &, long &, Error *, int); template void utils::bounds<>(const char *, int, const std::string &, - bigint, bigint, long long &, long long &, Error *); + bigint, bigint, long long &, long long &, Error *, int); // clang-format on /* ---------------------------------------------------------------------- @@ -768,7 +770,7 @@ template void utils::bounds_typelabel<>(const char *, int, const std::string &, ------------------------------------------------------------------------- */ int utils::expand_args(const char *file, int line, int narg, char **arg, int mode, char **&earg, - LAMMPS *lmp) + LAMMPS *lmp, int **argmap) { int iarg; @@ -783,10 +785,16 @@ int utils::expand_args(const char *file, int line, int narg, char **arg, int mod return narg; } + // determine argument offset + int ioffset = 0; + for (int i = 0; i < lmp->input->narg; ++i) + if (lmp->input->arg[i] == arg[0]) ioffset = i; + // maxarg should always end up equal to newarg, so caller can free earg int maxarg = narg - iarg; - earg = (char **) lmp->memory->smalloc(maxarg * sizeof(char *), "input:earg"); + earg = (char **) lmp->memory->smalloc(maxarg * sizeof(char *), "expand_args:earg"); + int *amap = (int *) lmp->memory->smalloc(maxarg * sizeof(int), "expand_args:amap"); int newarg = 0, expandflag, nlo, nhi, nmax; std::string id, wc, tail; @@ -849,16 +857,18 @@ int utils::expand_args(const char *file, int line, int narg, char **arg, int mod // expand wild card string to nlo/nhi numbers if (expandflag) { - utils::bounds(file, line, wc, 1, nmax, nlo, nhi, lmp->error); + utils::bounds(file, line, wc, 1, nmax, nlo, nhi, lmp->error, iarg + ioffset); if (newarg + nhi - nlo + 1 > maxarg) { maxarg += nhi - nlo + 1; - earg = (char **) lmp->memory->srealloc(earg, maxarg * sizeof(char *), "input:earg"); + earg = (char **) lmp->memory->srealloc(earg, maxarg * sizeof(char *), "expand_args:earg"); + amap = (int *) lmp->memory->srealloc(amap, maxarg * sizeof(char *), "expand_args:amap"); } for (int index = nlo; index <= nhi; index++) { earg[newarg] = utils::strdup(fmt::format("{}:{}:{}[{}]{}", gridid[0], gridid[1], id, index, tail)); + amap[newarg] = iarg; newarg++; } } @@ -936,7 +946,7 @@ int utils::expand_args(const char *file, int line, int narg, char **arg, int mod if (index >= 0) { if (mode == 0 && lmp->input->variable->vectorstyle(index)) { - utils::bounds(file, line, wc, 1, MAXSMALLINT, nlo, nhi, lmp->error); + utils::bounds(file, line, wc, 1, MAXSMALLINT, nlo, nhi, lmp->error, iarg + ioffset); if (nhi < MAXSMALLINT) { nmax = nhi; expandflag = 1; @@ -968,11 +978,12 @@ int utils::expand_args(const char *file, int line, int narg, char **arg, int mod // expand wild card string to nlo/nhi numbers - utils::bounds(file, line, wc, 1, nmax, nlo, nhi, lmp->error); + utils::bounds(file, line, wc, 1, nmax, nlo, nhi, lmp->error, iarg + ioffset); if (newarg + nhi - nlo + 1 > maxarg) { maxarg += nhi - nlo + 1; - earg = (char **) lmp->memory->srealloc(earg, maxarg * sizeof(char *), "input:earg"); + earg = (char **) lmp->memory->srealloc(earg, maxarg * sizeof(char *), "expand_args:earg"); + amap = (int *) lmp->memory->srealloc(amap, maxarg * sizeof(char *), "expand_args:amap"); } for (int index = nlo; index <= nhi; index++) { @@ -980,6 +991,7 @@ int utils::expand_args(const char *file, int line, int narg, char **arg, int mod earg[newarg] = utils::strdup(fmt::format("{}2_{}[{}]{}", word[0], id, index, tail)); else earg[newarg] = utils::strdup(fmt::format("{}_{}[{}]{}", word[0], id, index, tail)); + amap[newarg] = iarg; newarg++; } } @@ -990,14 +1002,17 @@ int utils::expand_args(const char *file, int line, int narg, char **arg, int mod if (!expandflag) { if (newarg == maxarg) { maxarg++; - earg = (char **) lmp->memory->srealloc(earg, maxarg * sizeof(char *), "input:earg"); + earg = (char **) lmp->memory->srealloc(earg, maxarg * sizeof(char *), "expand_args:earg"); + amap = (int *) lmp->memory->srealloc(amap, maxarg * sizeof(char *), "expand_args:amap"); } earg[newarg] = utils::strdup(word); + amap[newarg] = iarg; newarg++; } } - // printf("NEWARG %d\n",newarg); for (int i = 0; i < newarg; i++) printf(" arg %d: %s\n",i,earg[i]); + if (argmap && *argmap) *argmap = amap; + // fprintf(stderr, "NEWARG %d\n",newarg); for (int i = 0; i < newarg; i++) printf(" arg %d: %s %d\n",i,earg[i], amap ? amap[i] : -1); return newarg; } diff --git a/src/utils.h b/src/utils.h index 63f0643616..1854c07cf5 100644 --- a/src/utils.h +++ b/src/utils.h @@ -336,11 +336,12 @@ namespace utils { * \param nmax largest allowed upper bound * \param nlo lower bound * \param nhi upper bound - * \param error pointer to Error class for out-of-bounds messages */ + * \param error pointer to Error class for out-of-bounds messages + * \param failed argument index with failed expansion (optional) */ template void bounds(const char *file, int line, const std::string &str, bigint nmin, bigint nmax, - TYPE &nlo, TYPE &nhi, Error *error); + TYPE &nlo, TYPE &nhi, Error *error, int failed = -2); // -2 = Error::NOPOINTER /*! Same as utils::bounds(), but string may be a typelabel * @@ -386,17 +387,23 @@ This functions adds the following case to :cpp:func:`utils::bounds() Date: Fri, 17 Jan 2025 13:14:51 -0500 Subject: [PATCH 27/41] move misplaced break statement --- src/EXTRA-PAIR/pair_dispersion_d3.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EXTRA-PAIR/pair_dispersion_d3.cpp b/src/EXTRA-PAIR/pair_dispersion_d3.cpp index 1c2c075b35..ed48d88c07 100644 --- a/src/EXTRA-PAIR/pair_dispersion_d3.cpp +++ b/src/EXTRA-PAIR/pair_dispersion_d3.cpp @@ -1133,8 +1133,8 @@ void PairDispersionD3::set_funcpar(std::string &functional_name) a1 = 0.3065; s8 = 0.9147; a2 = 5.0570; - break; s6 = 0.64; + break; case 17: a1 = 0.0000; s8 = 0.2130; From 9b443c9a4dc8846ff1cc255abee19909800343cb Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 17 Jan 2025 14:06:30 -0500 Subject: [PATCH 28/41] add utility function to compare two string while ignoring whitespace --- doc/src/Developer_utils.rst | 3 +++ src/utils.cpp | 23 +++++++++++++++++++++++ src/utils.h | 8 ++++++++ unittest/utils/test_utils.cpp | 16 ++++++++++++++++ 4 files changed, 50 insertions(+) diff --git a/doc/src/Developer_utils.rst b/doc/src/Developer_utils.rst index 402ed680f7..2f9dce86d4 100644 --- a/doc/src/Developer_utils.rst +++ b/doc/src/Developer_utils.rst @@ -166,6 +166,9 @@ and parsing files or arguments. .. doxygenfunction:: split_lines :project: progguide +.. doxygenfunction:: strsame + :project: progguide + .. doxygenfunction:: strmatch :project: progguide diff --git a/src/utils.cpp b/src/utils.cpp index 90d165202a..b72a9f1be3 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -111,6 +111,29 @@ bool utils::strmatch(const std::string &text, const std::string &pattern) return (pos >= 0); } +bool utils::strsame(const std::string &text1, const std::string &text2) +{ + const char *ptr1 = text1.c_str(); + const char *ptr2 = text2.c_str(); + + while (*ptr1 && *ptr2) { + + // ignore whitespace + while (*ptr1 && isblank(*ptr1)) ++ptr1; + while (*ptr2 && isblank(*ptr2)) ++ptr2; + + // strings differ + if (*ptr1 != *ptr2) return false; + + // reached end of both strings + if (!*ptr1 && !*ptr2) return true; + + ++ptr1; + ++ptr2; + } + return true; +} + /** This function is a companion function to utils::strmatch(). Arguments * and logic is the same, but instead of a boolean, it returns the * sub-string that matches the regex pattern. There can be only one match. diff --git a/src/utils.h b/src/utils.h index 1854c07cf5..1d1ce27de5 100644 --- a/src/utils.h +++ b/src/utils.h @@ -41,6 +41,14 @@ namespace utils { bool strmatch(const std::string &text, const std::string &pattern); + /*! Compare two string while ignoring whitespace + * + * \param text1 the first text to be compared + * \param text2 the second text to be compared + * \return true if the non-whitespace part of the two strings matches, false if not */ + + bool strsame(const std::string &text1, const std::string &text2); + /*! Find sub-string that matches a simplified regex pattern * * \param text the text to be matched against the pattern diff --git a/unittest/utils/test_utils.cpp b/unittest/utils/test_utils.cpp index 510fcb0198..2a46de43f7 100644 --- a/unittest/utils/test_utils.cpp +++ b/unittest/utils/test_utils.cpp @@ -46,6 +46,22 @@ TEST(Utils, strdup) delete[] copy2; } +TEST(Utils, strsame) +{ + std::string text1("some_text"); + std::string text2("some_text"); + ASSERT_TRUE(utils::strsame(text1,text2)); + text1 = " some _\ttext\n "; + ASSERT_TRUE(utils::strsame(text1,text2)); + text2 = " some _ text\n "; + ASSERT_TRUE(utils::strsame(text1,text2)); + + text2 = "some_other_text"; + ASSERT_FALSE(utils::strsame(text1,text2)); + text2 = " some other_text"; + ASSERT_FALSE(utils::strsame(text1,text2)); +} + TEST(Utils, trim) { auto trimmed = utils::trim("\t some text"); From e350f28e26a99785a8fdbf8ec3976f9f08c06d7c Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 17 Jan 2025 18:06:31 -0500 Subject: [PATCH 29/41] refactor how error output is created and only print input and parsed line if they differ in text --- src/error.cpp | 29 +++------------- src/fix_ave_time.cpp | 2 +- src/input.h | 1 + src/utils.cpp | 82 +++++++++++++++++++++++++++++--------------- src/utils.h | 4 +-- 5 files changed, 63 insertions(+), 55 deletions(-) diff --git a/src/error.cpp b/src/error.cpp index d03e5453e9..6dbd0a20b3 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -121,16 +121,9 @@ void Error::all(const std::string &file, int line, int failed, const std::string std::string lastcmd = "(unknown)"; std::string mesg = "ERROR: " + str + fmt::format(" ({}:{})\n", truncpath(file), line); - if (input && input->line) lastcmd = input->line; + // add text about the input following the error message - if (failed > NOLASTLINE) { - try { - mesg += fmt::format("Last input line: {}\n", lastcmd); - if (failed > NOPOINTER) mesg += utils::point_to_error(input, failed); - } catch (fmt::format_error &) { - ; // do nothing - } - } + if (failed > NOLASTLINE) mesg += utils::point_to_error(input, failed); if (comm->me == 0) utils::logmesg(lmp,mesg); // allow commands if an exception was caught in a run @@ -153,23 +146,11 @@ void Error::all(const std::string &file, int line, int failed, const std::string void Error::one(const std::string &file, int line, int failed, const std::string &str) { - int me; std::string lastcmd = "(unknown)"; - MPI_Comm_rank(world,&me); - if (input && input->line) lastcmd = input->line; - std::string mesg; - try { - if (failed == NOLASTLINE) { - mesg = fmt::format("ERROR on proc {}: {} ({}:{})\n", me, str, truncpath(file), line); - } else { - mesg = fmt::format("ERROR on proc {}: {} ({}:{})\nLast input line: {}\n", - me, str, truncpath(file), line, lastcmd); - } - if (failed > NOPOINTER) mesg += utils::point_to_error(input, failed); - } catch (fmt::format_error &) { - ; // do nothing - } + std::string mesg = fmt::format("ERROR on proc {}: {} ({}:{})\n", comm->me, str, + truncpath(file), line); + if (failed > NOPOINTER) mesg += utils::point_to_error(input, failed); utils::logmesg(lmp,mesg); if (universe->nworlds > 1) diff --git a/src/fix_ave_time.cpp b/src/fix_ave_time.cpp index 8c7d902b86..7efb8cc8a6 100644 --- a/src/fix_ave_time.cpp +++ b/src/fix_ave_time.cpp @@ -1075,7 +1075,7 @@ void FixAveTime::options(int iarg, int narg, char **arg) if (iarg+2 > narg) utils::missing_cmd_args(FLERR, "fix ave/time mode", error); if (strcmp(arg[iarg+1],"scalar") == 0) mode = SCALAR; else if (strcmp(arg[iarg+1],"vector") == 0) mode = VECTOR; - else error->all(FLERR,"Unknown fix ave/time mode {}", arg[iarg+1]); + else error->all(FLERR,iarg+1,"Unknown fix ave/time mode {}", arg[iarg+1]); iarg += 2; } else if (strcmp(arg[iarg],"off") == 0) { if (iarg+2 > narg) utils::missing_cmd_args(FLERR, "fix ave/time off", error); diff --git a/src/input.h b/src/input.h index e71ee22174..728c224835 100644 --- a/src/input.h +++ b/src/input.h @@ -26,6 +26,7 @@ class Input : protected Pointers { friend class Error; friend class Deprecated; friend class SimpleCommandsTest_Echo_Test; + friend std::string utils::point_to_error(Input *input, int failed); public: char *command; // ptr to current command diff --git a/src/utils.cpp b/src/utils.cpp index b72a9f1be3..4f1a0cb3fa 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -158,39 +158,65 @@ void utils::missing_cmd_args(const std::string &file, int line, const std::strin std::string utils::point_to_error(Input *input, int failed) { if (input) { - std::string cmdline = "--> parsed line: "; - int indicator = cmdline.size(); // error indicator points to command by default - cmdline += input->command; - cmdline += ' '; + std::string lastline = input->line; + std::string lastargs = input->command; + std::string cmdline = "Last input line: "; - // assemble pre-processed command line and update error indicator position, if needed. - for (int i = 0; i < input->narg; ++i) { - std::string inputarg = input->arg[i]; - if (i == failed) indicator = cmdline.size(); + // extended output + if (failed > Error::NOPOINTER) { - // argument contains whitespace. add quotes. check which type of quotes, too - if (inputarg.find_first_of(" \t\n") != std::string::npos) { - if (inputarg.find_first_of('"') != std::string::npos) { - cmdline += "'"; - cmdline += inputarg; - cmdline += "'"; - } else { - cmdline += '"'; - cmdline += inputarg; - cmdline += '"'; - } - } else - cmdline += inputarg; - cmdline += ' '; + // indicator points to command by default + int indicator = 0; + int quoted = 0; + lastargs += ' '; + + // assemble pre-processed command line and update error indicator position, if needed. + for (int i = 0; i < input->narg; ++i) { + std::string inputarg = input->arg[i]; + if (i == failed) indicator = lastargs.size(); + + // argument contains whitespace. add quotes. check which type of quotes, too + if (inputarg.find_first_of(" \t\n") != std::string::npos) { + if (i == failed) quoted = 2; + if (inputarg.find_first_of('"') != std::string::npos) { + lastargs += "'"; + lastargs += inputarg; + lastargs += "'"; + } else { + lastargs += '"'; + lastargs += inputarg; + lastargs += '"'; + } + } else + lastargs += inputarg; + lastargs += ' '; + } + + indicator += cmdline.size(); + // the string is unchanged by substitution (ignoring whitespace), print output only once + if (utils::strsame(lastline, lastargs)) { + cmdline += lastargs; + } else { + cmdline += lastline; + cmdline += '\n'; + // must have the same number of chars as "Last input line: " used in the previous line + cmdline += "--> parsed line: "; + cmdline += lastargs; + } + + // construct and append error indicator line + cmdline += '\n'; + cmdline += std::string(indicator, ' '); + cmdline += std::string(strlen(input->arg[failed]) + quoted, '^'); + cmdline += '\n'; + + } else { + cmdline += input->line; + cmdline += '\n'; } - // construct and append error indicator line - cmdline += '\n'; - cmdline += std::string(indicator, ' '); - cmdline += std::string(strlen(input->arg[failed]), '^'); - cmdline += '\n'; return cmdline; } else - return std::string("(Failed command line text not available)"); + return std::string("(Failed input line text not available)"); } /* specialization for the case of just a single string argument */ diff --git a/src/utils.h b/src/utils.h index 1d1ce27de5..598ff56eb3 100644 --- a/src/utils.h +++ b/src/utils.h @@ -68,7 +68,7 @@ namespace utils { void missing_cmd_args(const std::string &file, int line, const std::string &cmd, Error *error); - /*! Create string with last command after pre-processing and pointing to arg with error + /*! Create string with last command and optionally pointing to arg with error * * This function is a helper function for error messages. It creates * @@ -349,7 +349,7 @@ namespace utils { template void bounds(const char *file, int line, const std::string &str, bigint nmin, bigint nmax, - TYPE &nlo, TYPE &nhi, Error *error, int failed = -2); // -2 = Error::NOPOINTER + TYPE &nlo, TYPE &nhi, Error *error, int failed = -2); // -2 = Error::NOPOINTER /*! Same as utils::bounds(), but string may be a typelabel * From e38c13a76435bfccf274636c144634b33211c29f Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 17 Jan 2025 22:59:44 -0500 Subject: [PATCH 30/41] add strcompress function and use it for error output --- doc/src/Developer_utils.rst | 3 +++ src/utils.cpp | 32 +++++++++++++++++++++++++++--- src/utils.h | 15 ++++++++++---- unittest/utils/test_utils.cpp | 37 ++++++++++++++++++++++++++++++----- 4 files changed, 75 insertions(+), 12 deletions(-) diff --git a/doc/src/Developer_utils.rst b/doc/src/Developer_utils.rst index 2f9dce86d4..86a536f0cd 100644 --- a/doc/src/Developer_utils.rst +++ b/doc/src/Developer_utils.rst @@ -133,6 +133,9 @@ and parsing files or arguments. .. doxygenfunction:: trim_comment :project: progguide +.. doxygenfunction:: strcompress + :project: progguide + .. doxygenfunction:: strip_style_suffix :project: progguide diff --git a/src/utils.cpp b/src/utils.cpp index 4f1a0cb3fa..71a662a7e9 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -119,8 +119,8 @@ bool utils::strsame(const std::string &text1, const std::string &text2) while (*ptr1 && *ptr2) { // ignore whitespace - while (*ptr1 && isblank(*ptr1)) ++ptr1; - while (*ptr2 && isblank(*ptr2)) ++ptr2; + while (*ptr1 && isspace(*ptr1)) ++ptr1; + while (*ptr2 && isspace(*ptr2)) ++ptr2; // strings differ if (*ptr1 != *ptr2) return false; @@ -134,6 +134,32 @@ bool utils::strsame(const std::string &text1, const std::string &text2) return true; } +std::string utils::strcompress(const std::string &text) +{ + const char *ptr = text.c_str(); + std::string output; + + // remove leading whitespace + while (*ptr && isspace(*ptr)) ++ptr; + + while (*ptr) { + // copy non-blank characters + while (*ptr && !isspace(*ptr)) output += *ptr++; + + if (!*ptr) break; + + // add one blank only + if (isspace(*ptr)) output += ' '; + + // skip additional blanks + while (*ptr && isspace(*ptr)) ++ptr; + } + + // remove trailing blank + if (output.back() == ' ') output.erase(output.size() - 1, 1); + return output; +} + /** This function is a companion function to utils::strmatch(). Arguments * and logic is the same, but instead of a boolean, it returns the * sub-string that matches the regex pattern. There can be only one match. @@ -158,7 +184,7 @@ void utils::missing_cmd_args(const std::string &file, int line, const std::strin std::string utils::point_to_error(Input *input, int failed) { if (input) { - std::string lastline = input->line; + std::string lastline = utils::strcompress(input->line); std::string lastargs = input->command; std::string cmdline = "Last input line: "; diff --git a/src/utils.h b/src/utils.h index 598ff56eb3..21300c4943 100644 --- a/src/utils.h +++ b/src/utils.h @@ -35,20 +35,27 @@ namespace utils { /*! Match text against a simplified regex pattern * - * \param text the text to be matched against the pattern - * \param pattern the search pattern, which may contain regexp markers + * \param text the text to be matched against the pattern + * \param pattern the search pattern, which may contain regexp markers * \return true if the pattern matches, false if not */ bool strmatch(const std::string &text, const std::string &pattern); /*! Compare two string while ignoring whitespace * - * \param text1 the first text to be compared - * \param text2 the second text to be compared + * \param text1 the first text to be compared + * \param text2 the second text to be compared * \return true if the non-whitespace part of the two strings matches, false if not */ bool strsame(const std::string &text1, const std::string &text2); + /*! Compress whitespace in a string + * + * \param text the text to be compressed + * \return string with whitespace compressed to single blanks */ + + std::string strcompress(const std::string &text); + /*! Find sub-string that matches a simplified regex pattern * * \param text the text to be matched against the pattern diff --git a/unittest/utils/test_utils.cpp b/unittest/utils/test_utils.cpp index 2a46de43f7..cb6fb9c05b 100644 --- a/unittest/utils/test_utils.cpp +++ b/unittest/utils/test_utils.cpp @@ -50,16 +50,43 @@ TEST(Utils, strsame) { std::string text1("some_text"); std::string text2("some_text"); - ASSERT_TRUE(utils::strsame(text1,text2)); + ASSERT_TRUE(utils::strsame(text1, text2)); text1 = " some _\ttext\n "; - ASSERT_TRUE(utils::strsame(text1,text2)); + ASSERT_TRUE(utils::strsame(text1, text2)); text2 = " some _ text\n "; - ASSERT_TRUE(utils::strsame(text1,text2)); + ASSERT_TRUE(utils::strsame(text1, text2)); text2 = "some_other_text"; - ASSERT_FALSE(utils::strsame(text1,text2)); + ASSERT_FALSE(utils::strsame(text1, text2)); text2 = " some other_text"; - ASSERT_FALSE(utils::strsame(text1,text2)); + ASSERT_FALSE(utils::strsame(text1, text2)); +} + +TEST(Utils, strcompress) +{ + auto compressed = utils::strcompress("\t some text "); + ASSERT_THAT(compressed, StrEq("some text")); + + compressed = utils::strcompress("some \ntext"); + ASSERT_THAT(compressed, StrEq("some text")); + + compressed = utils::strcompress("sometext"); + ASSERT_THAT(compressed, StrEq("sometext")); + + compressed = utils::strcompress("some text \r\n"); + ASSERT_THAT(compressed, StrEq("some text")); + + compressed = utils::strcompress("some other text \r\n"); + ASSERT_THAT(compressed, StrEq("some other text")); + + compressed = utils::strcompress("\v some text \f"); + ASSERT_THAT(compressed, StrEq("some text")); + + compressed = utils::strcompress(" some\t text "); + ASSERT_THAT(compressed, StrEq("some text")); + + compressed = utils::strcompress(" \t\n "); + ASSERT_THAT(compressed, StrEq("")); } TEST(Utils, trim) From efd5165707d37479c42cb7e2e87130c0662f32e3 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 17 Jan 2025 23:27:36 -0500 Subject: [PATCH 31/41] avoid segfaults if there was no input processing --- src/utils.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils.cpp b/src/utils.cpp index 71a662a7e9..0007b11e9c 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -183,7 +183,7 @@ void utils::missing_cmd_args(const std::string &file, int line, const std::strin std::string utils::point_to_error(Input *input, int failed) { - if (input) { + if (input && input->line && input->command) { std::string lastline = utils::strcompress(input->line); std::string lastargs = input->command; std::string cmdline = "Last input line: "; @@ -237,12 +237,12 @@ std::string utils::point_to_error(Input *input, int failed) cmdline += '\n'; } else { - cmdline += input->line; + cmdline += lastline; cmdline += '\n'; } return cmdline; } else - return std::string("(Failed input line text not available)"); + return std::string(""); } /* specialization for the case of just a single string argument */ From 7ffe04ca9208af95ca0213fbce378a3048e1a715 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 18 Jan 2025 09:08:08 -0500 Subject: [PATCH 32/41] discuss that only turning off bonds can lead to "bond atom missing" errors also make notes and this warning stand out more by using adminition boxes --- doc/src/delete_bonds.rst | 41 ++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/doc/src/delete_bonds.rst b/doc/src/delete_bonds.rst index e03c4b3ac7..e6825ded33 100644 --- a/doc/src/delete_bonds.rst +++ b/doc/src/delete_bonds.rst @@ -62,6 +62,18 @@ For all styles, by default, an interaction is only turned off (or on) if all the atoms involved are in the specified group. See the *any* keyword to change the behavior. +.. admonition:: Possible errors caused by using *delete_bonds* + :class: warning + + Since this command by default only *turns off* bonded interactions, + their definitions are still present and subject to the limitations + due to LAMMPS' domain decomposition based parallelization. That is, + when a bond is turned off, the two constituent atoms may move apart + and may reach a distance where they can lead to a "bond atoms missing" + error and crash the simulation. Adding the *remove* keyword (see + below) is required to fully remove those interactions and prevent + the error. + Several of the styles (\ *atom*, *bond*, *angle*, *dihedral*, *improper*\ ) take a *type* as an argument. The specified *type* can be a :doc:`type label `. Otherwise, the type should be an @@ -98,15 +110,18 @@ of all interactions in the specified group is simply reported. This is useful for diagnostic purposes if bonds have been turned off by a bond-breaking potential during a previous run. -The default behavior of the delete_bonds command is to turn off -interactions by toggling their type to a negative value, but not to -permanently remove the interaction. For example, a bond_type of 2 is set to -:math:`-2.` The neighbor list creation routines will not include such an -interaction in their interaction lists. The default is also to not -alter the list of 1--2, 1--3, or 1--4 neighbors computed by the -:doc:`special_bonds ` command and used to weight pairwise -force and energy calculations. This means that pairwise computations -will proceed as if the bond (or angle, etc.) were still turned on. +.. admonition:: Impact on special_bonds processing and exclusions + :class: note + + The default behavior of the delete_bonds command is to turn off + interactions by toggling their type to a negative value, but not to + permanently remove the interaction. For example, a bond_type of 2 is set to + :math:`-2.` The neighbor list creation routines will not include such an + interaction in their interaction lists. The default is also to not + alter the list of 1--2, 1--3, or 1--4 neighbors computed by the + :doc:`special_bonds ` command and used to weight pairwise + force and energy calculations. This means that pairwise computations + will proceed as if the bond (or angle, etc.) were still turned on. Several keywords can be appended to the argument list to alter the default behaviors. @@ -138,9 +153,11 @@ operation, after (optional) removal. It re-computes the pairwise 1--2, turned-off bonds the same as turned-on. Thus, turned-off bonds must be removed if you wish to change the weighting list. -Note that the choice of *remove* and *special* options affects how -1--2, 1--3, 1--4 pairwise interactions will be computed across bonds that -have been modified by the delete_bonds command. +.. note:: + + The choice of *remove* and *special* options affects how 1--2, + 1--3, 1--4 pairwise interactions will be computed across bonds + that have been modified by the delete_bonds command. Restrictions """""""""""" From 23c63511f08b71e74b54b9b79efa43c6bca5cf9a Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 18 Jan 2025 11:08:25 -0500 Subject: [PATCH 33/41] no hiden tabs, use string escapes instead. --- unittest/utils/test_utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/utils/test_utils.cpp b/unittest/utils/test_utils.cpp index cb6fb9c05b..8bfb628f3e 100644 --- a/unittest/utils/test_utils.cpp +++ b/unittest/utils/test_utils.cpp @@ -79,7 +79,7 @@ TEST(Utils, strcompress) compressed = utils::strcompress("some other text \r\n"); ASSERT_THAT(compressed, StrEq("some other text")); - compressed = utils::strcompress("\v some text \f"); + compressed = utils::strcompress("\v some \t\t text \f"); ASSERT_THAT(compressed, StrEq("some text")); compressed = utils::strcompress(" some\t text "); From 7b994801b576bff227f625167d412427ec430688 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 18 Jan 2025 11:37:20 -0500 Subject: [PATCH 34/41] constant was denormal. increase to become normal --- src/EXTRA-MOLECULE/angle_gaussian.cpp | 2 +- src/EXTRA-MOLECULE/bond_gaussian.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/EXTRA-MOLECULE/angle_gaussian.cpp b/src/EXTRA-MOLECULE/angle_gaussian.cpp index a5d469559f..004fdd15ab 100644 --- a/src/EXTRA-MOLECULE/angle_gaussian.cpp +++ b/src/EXTRA-MOLECULE/angle_gaussian.cpp @@ -29,7 +29,7 @@ using namespace LAMMPS_NS; using namespace MathConst; static constexpr double SMALL = 0.001; -static constexpr double SMALLG = 2.0e-308; +static constexpr double SMALLG = 2.3e-308; /* ---------------------------------------------------------------------- */ diff --git a/src/EXTRA-MOLECULE/bond_gaussian.cpp b/src/EXTRA-MOLECULE/bond_gaussian.cpp index 2ed9e06799..deb37042ad 100644 --- a/src/EXTRA-MOLECULE/bond_gaussian.cpp +++ b/src/EXTRA-MOLECULE/bond_gaussian.cpp @@ -27,7 +27,7 @@ using namespace LAMMPS_NS; using namespace MathConst; -static constexpr double SMALL = 2.0e-308; +static constexpr double SMALL = 2.3e-308; /* ---------------------------------------------------------------------- */ From 0501f76fcf67c7203adeb2ece1b98a3ae05eaa89 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 18 Jan 2025 11:37:41 -0500 Subject: [PATCH 35/41] follow LAMMPS programming style more closely --- src/EXTRA-PAIR/pair_dispersion_d3.cpp | 6 +++- src/EXTRA-PAIR/pair_dispersion_d3.h | 46 +++++++++++---------------- src/LEPTON/fix_efield_lepton.cpp | 7 ++-- src/pair.cpp | 9 +++--- 4 files changed, 33 insertions(+), 35 deletions(-) diff --git a/src/EXTRA-PAIR/pair_dispersion_d3.cpp b/src/EXTRA-PAIR/pair_dispersion_d3.cpp index ed48d88c07..ae1ec4dc25 100644 --- a/src/EXTRA-PAIR/pair_dispersion_d3.cpp +++ b/src/EXTRA-PAIR/pair_dispersion_d3.cpp @@ -62,7 +62,9 @@ static constexpr double autoev = 27.21140795; // atomic units (Hartree) to eV Constructor (Required) ------------------------------------------------------------------------- */ -PairDispersionD3::PairDispersionD3(LAMMPS *lmp) : Pair(lmp) +PairDispersionD3::PairDispersionD3(LAMMPS *lmp) : + Pair(lmp), r2r4(nullptr), rcov(nullptr), mxci(nullptr), r0ab(nullptr), c6ab(nullptr), + cn(nullptr), dc6(nullptr) { nmax = 0; comm_forward = 2; @@ -72,6 +74,8 @@ PairDispersionD3::PairDispersionD3(LAMMPS *lmp) : Pair(lmp) manybody_flag = 1; one_coeff = 1; single_enable = 0; + + s6 = s8 = s18 = rs6 = rs8 = rs18 = a1 = a2 = alpha = alpha6 = alpha8 = 0.0; } /* ---------------------------------------------------------------------- diff --git a/src/EXTRA-PAIR/pair_dispersion_d3.h b/src/EXTRA-PAIR/pair_dispersion_d3.h index 58575c8a2e..32d1fadec1 100644 --- a/src/EXTRA-PAIR/pair_dispersion_d3.h +++ b/src/EXTRA-PAIR/pair_dispersion_d3.h @@ -27,12 +27,10 @@ namespace LAMMPS_NS { class PairDispersionD3 : public Pair { public: - PairDispersionD3(class LAMMPS *); ~PairDispersionD3() override; void compute(int, int) override; - void settings(int, char **) override; void coeff(int, char **) override; void init_style() override; @@ -45,45 +43,39 @@ class PairDispersionD3 : public Pair { void unpack_reverse_comm(int, int *, double *) override; protected: - int nmax; - double evdwl; - double rthr; // R^2 distance to cutoff for D3_calculation - double cn_thr; // R^2 distance to cutoff for CN_calculation + double rthr; // R^2 distance to cutoff for D3_calculation + double cn_thr; // R^2 distance to cutoff for CN_calculation - std::string damping_type; // damping function type - double s6, s8, s18, rs6, rs8, rs18; // XC parameters + std::string damping_type; // damping function type + double s6, s8, s18, rs6, rs8, rs18; // XC parameters double a1, a2, alpha, alpha6, alpha8; - double* r2r4 = nullptr; // scale r4/r2 values of the atoms by sqrt(Z) - double* rcov = nullptr; // covalent radii - int* mxci = nullptr; // How large the grid for c6 interpolation + double *r2r4; // scale r4/r2 values of the atoms by sqrt(Z) + double *rcov; // covalent radii + int *mxci; // How large the grid for c6 interpolation + double **r0ab; // cut-off radii for all element pairs + double *****c6ab; // C6 for all element pairs + double *cn; // Coordination numbers + double *dc6; // dC6i(iat) saves dE_dsp/dCN(iat) - double** r0ab = nullptr; // cut-off radii for all element pairs - double***** c6ab = nullptr; // C6 for all element pairs - - double* cn = nullptr; // Coordination numbers - double* dc6 = nullptr; // dC6i(iat) saves dE_dsp/dCN(iat) - - int communicationStage; // communication stage + int communicationStage; // communication stage void allocate(); - virtual void set_funcpar(std::string&); + virtual void set_funcpar(std::string &); void calc_coordination_number(); - int find_atomic_number(std::string&); - std::vector is_int_in_array(int*, int, int); + int find_atomic_number(std::string &); + std::vector is_int_in_array(int *, int, int); - void read_r0ab(int*, int); - void set_limit_in_pars_array(int&, int&, int&, int&); - void read_c6ab(int*, int); + void read_r0ab(int *, int); + void set_limit_in_pars_array(int &, int &, int &, int &); + void read_c6ab(int *, int); - double* get_dC6(int, int, double, double); + double *get_dC6(int, int, double, double); }; - } // namespace LAMMPS_NS - #endif #endif diff --git a/src/LEPTON/fix_efield_lepton.cpp b/src/LEPTON/fix_efield_lepton.cpp index d3b82b40c5..a45305aee3 100644 --- a/src/LEPTON/fix_efield_lepton.cpp +++ b/src/LEPTON/fix_efield_lepton.cpp @@ -58,6 +58,9 @@ FixEfieldLepton::FixEfieldLepton(LAMMPS *lmp, int narg, char **arg) : respa_level_support = 1; ilevel_respa = 0; + qe2f = force->qe2f; + mue2e = qe2f; + // optional args int iarg = 4; while (iarg < narg) { @@ -133,10 +136,8 @@ void FixEfieldLepton::init() if (respa_level >= 0) ilevel_respa = MIN(respa_level, ilevel_respa); } - // unit conversion factors and restrictions (see issue #1377) + // unit conversion restrictions (see issue #1377) char *unit_style = update->unit_style; - qe2f = force->qe2f; - mue2e = qe2f; if (strcmp(unit_style, "electron") == 0 || strcmp(unit_style, "micro") == 0 || strcmp(unit_style, "nano") == 0) { error->all(FLERR, "Fix {} does not support {} units", style, unit_style); diff --git a/src/pair.cpp b/src/pair.cpp index 5421108eba..8e9a4e44be 100644 --- a/src/pair.cpp +++ b/src/pair.cpp @@ -710,10 +710,11 @@ double Pair::mix_energy(double eps1, double eps2, double sig1, double sig2) return sqrt(eps1*eps2); else if (mix_flag == ARITHMETIC) return sqrt(eps1*eps2); - else if (mix_flag == SIXTHPOWER) - return (2.0 * sqrt(eps1*eps2) * powint(sig1, 3) * powint(sig2, 3) - / (powint(sig1, 6) + powint(sig2, 6))); - else did_mix = false; + else if (mix_flag == SIXTHPOWER) { + if ((sig1 != 0.0) && (sig2 != 0.0)) + return (2.0 * sqrt(eps1*eps2) * powint(sig1, 3) * powint(sig2, 3) + / (powint(sig1, 6) + powint(sig2, 6))); + } else did_mix = false; return 0.0; } From c2bcf791967727449d9a93e465df161a03c8e87e Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 18 Jan 2025 11:45:27 -0500 Subject: [PATCH 36/41] avoid divisions --- src/ML-POD/eapod.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/ML-POD/eapod.cpp b/src/ML-POD/eapod.cpp index 11dace1f28..0525cfdb55 100644 --- a/src/ML-POD/eapod.cpp +++ b/src/ML-POD/eapod.cpp @@ -982,10 +982,14 @@ double EAPOD::peratom_environment_descriptors(double *cb, double *bd, double *tm D[j] = 1.0 / sum; } - double sum = 0; + double sum = 0.0; for (int j = 0; j < nClusters; j++) sum += D[j]; double sumD = sum; - for (int j = 0; j < nClusters; j++) P[j] = D[j]/sum; + if (sum != 0.0) { + for (int j = 0; j < nClusters; j++) P[j] = D[j]/sum; + } else { + for (int j = 0; j < nClusters; j++) P[j] = 0.0; + } int nc = nCoeffPerElement*(ti[0]-1); double ei = coeff[0 + nc]; @@ -1008,13 +1012,13 @@ double EAPOD::peratom_environment_descriptors(double *cb, double *bd, double *tm } for (int m = 0; m Date: Sat, 18 Jan 2025 11:54:48 -0500 Subject: [PATCH 37/41] make coverity scan happy --- src/SHOCK/fix_wall_piston.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/SHOCK/fix_wall_piston.cpp b/src/SHOCK/fix_wall_piston.cpp index cb20e2d683..76cdf1f3b8 100644 --- a/src/SHOCK/fix_wall_piston.cpp +++ b/src/SHOCK/fix_wall_piston.cpp @@ -39,10 +39,8 @@ FixWallPiston::FixWallPiston(LAMMPS *lmp, int narg, char **arg) : force_reneighbor = 1; next_reneighbor = -1; - if (narg < 4) error->all(FLERR,"Illegal fix wall/piston command"); + if (narg < 4) utils::missing_cmd_args(FLERR,"fix wall/piston", error); - randomt = nullptr; - gfactor1 = gfactor2 = nullptr; tempflag = 0; scaleflag = 1; roughflag = 0; @@ -92,6 +90,9 @@ FixWallPiston::FixWallPiston(LAMMPS *lmp, int narg, char **arg) : if (t_period <= 0) error->all(FLERR,"Illegal fix wall/piston command"); if (t_extent <= 0) error->all(FLERR,"Illegal fix wall/piston command"); if (tseed <= 0) error->all(FLERR,"Illegal fix wall/piston command"); + delete randomt; + delete[] gfactor1; + delete[] gfactor2; randomt = new RanMars(lmp,tseed + comm->me); gfactor1 = new double[atom->ntypes+1]; gfactor2 = new double[atom->ntypes+1]; From 9cca8ab179edb3d74e6384e7cee8b62c7c56403b Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 18 Jan 2025 15:54:34 -0500 Subject: [PATCH 38/41] modernize --- src/RIGID/compute_rigid_local.cpp | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/RIGID/compute_rigid_local.cpp b/src/RIGID/compute_rigid_local.cpp index ea45389e7b..38a5788b99 100644 --- a/src/RIGID/compute_rigid_local.cpp +++ b/src/RIGID/compute_rigid_local.cpp @@ -13,14 +13,16 @@ ------------------------------------------------------------------------- */ #include "compute_rigid_local.h" -#include + #include "atom.h" -#include "update.h" #include "domain.h" +#include "error.h" #include "modify.h" #include "fix_rigid_small.h" #include "memory.h" -#include "error.h" +#include "update.h" + +#include using namespace LAMMPS_NS; @@ -98,8 +100,8 @@ ComputeRigidLocal::~ComputeRigidLocal() { memory->destroy(vlocal); memory->destroy(alocal); - delete [] idrigid; - delete [] rstyle; + delete[] idrigid; + delete[] rstyle; } /* ---------------------------------------------------------------------- */ @@ -108,16 +110,11 @@ void ComputeRigidLocal::init() { // set fixrigid - int ifix = modify->find_fix(idrigid); - if (ifix < 0) - error->all(FLERR,"FixRigidSmall ID for compute rigid/local does not exist"); - fixrigid = dynamic_cast(modify->fix[ifix]); - - int flag = 0; - if (strstr(fixrigid->style,"rigid/") == nullptr) flag = 1; - if (strstr(fixrigid->style,"/small") == nullptr) flag = 1; - if (flag) - error->all(FLERR,"Compute rigid/local does not use fix rigid/small fix"); + auto ifix = modify->get_fix_by_id(idrigid); + if (!ifix) error->all(FLERR,"FixRigidSmall ID {} for compute rigid/local does not exist", idrigid); + fixrigid = dynamic_cast(ifix); + if (!fixrigid) + error->all(FLERR,"Fix ID {} for compute rigid/local does not point to fix rigid/small", idrigid); // do initial memory allocation so that memory_usage() is correct From d99c960eb9286df4dead0715ed827acb4af88b41 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 18 Jan 2025 21:40:48 -0500 Subject: [PATCH 39/41] resolve unit test failures due to enhanced error handling --- src/fix_ave_time.cpp | 6 ++++-- src/utils.cpp | 15 ++++++++++----- unittest/cplusplus/test_advanced_utils.cpp | 4 ++++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/fix_ave_time.cpp b/src/fix_ave_time.cpp index 7efb8cc8a6..3ac8e68cad 100644 --- a/src/fix_ave_time.cpp +++ b/src/fix_ave_time.cpp @@ -79,7 +79,7 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : int expand = 0; char **earg; - int *amap; + int *amap = nullptr; nvalues = utils::expand_args(FLERR,nvalues,&arg[6],mode,earg,lmp,&amap); key2col.clear(); @@ -101,7 +101,8 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : error->all(FLERR, amap[i]+6,"Invalid fix ave/time argument: {}", arg[i]); val.argindex = argi.get_index1(); - val.iarg = amap[i] + 6; + if (expand) val.iarg = amap[i] + 6; + else val.iarg = i + 6; val.varlen = 0; val.offcol = 0; val.id = argi.get_name(); @@ -278,6 +279,7 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : if (expand) { for (int i = 0; i < nvalues; i++) delete[] earg[i]; memory->sfree(earg); + memory->sfree(amap); } // allocate memory for averaging diff --git a/src/utils.cpp b/src/utils.cpp index 0007b11e9c..52d7e5113a 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -860,10 +860,12 @@ int utils::expand_args(const char *file, int line, int narg, char **arg, int mod return narg; } - // determine argument offset + // determine argument offset, if possible int ioffset = 0; - for (int i = 0; i < lmp->input->narg; ++i) - if (lmp->input->arg[i] == arg[0]) ioffset = i; + if (lmp->input->arg) { + for (int i = 0; i < lmp->input->narg; ++i) + if (lmp->input->arg[i] == arg[0]) ioffset = i; + } // maxarg should always end up equal to newarg, so caller can free earg @@ -1052,7 +1054,6 @@ int utils::expand_args(const char *file, int line, int narg, char **arg, int mod if (expandflag) { // expand wild card string to nlo/nhi numbers - utils::bounds(file, line, wc, 1, nmax, nlo, nhi, lmp->error, iarg + ioffset); if (newarg + nhi - nlo + 1 > maxarg) { @@ -1086,7 +1087,11 @@ int utils::expand_args(const char *file, int line, int narg, char **arg, int mod } } - if (argmap && *argmap) *argmap = amap; + if (argmap) + *argmap = amap; + else + lmp->memory->sfree(amap); + // fprintf(stderr, "NEWARG %d\n",newarg); for (int i = 0; i < newarg; i++) printf(" arg %d: %s %d\n",i,earg[i], amap ? amap[i] : -1); return newarg; } diff --git a/unittest/cplusplus/test_advanced_utils.cpp b/unittest/cplusplus/test_advanced_utils.cpp index 1da9500b35..468f353080 100644 --- a/unittest/cplusplus/test_advanced_utils.cpp +++ b/unittest/cplusplus/test_advanced_utils.cpp @@ -151,6 +151,10 @@ TEST_F(Advanced_utils, expand_args) args[7] = utils::strdup("c_gofr[*2][2]"); args[8] = utils::strdup("c_gofr[*][*]"); + // disable use of input->command and input->arg which point to the last run command right now + lmp->input->command = nullptr; + lmp->input->arg = nullptr; + auto narg = utils::expand_args(FLERR, oarg, args, 0, earg, lmp); EXPECT_EQ(narg, 16); EXPECT_STREQ(earg[0], "v_step"); From a5b66f02d19c50c93df1fbbbfb352535340ee692 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 18 Jan 2025 22:19:43 -0500 Subject: [PATCH 40/41] update docs --- doc/src/Developer_notes.rst | 21 +++++++++++++++------ doc/src/Run_output.rst | 35 ++++++++++++++++++++++++----------- src/utils.h | 14 +++++++------- 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/doc/src/Developer_notes.rst b/doc/src/Developer_notes.rst index a562ee26e4..2d136055a4 100644 --- a/doc/src/Developer_notes.rst +++ b/doc/src/Developer_notes.rst @@ -268,12 +268,14 @@ There are multiple "signatures" that can be called: command would be the last line, but is unrelated to the error. - ``Error::all(FLERR, idx, "Error message")``: this is for argument - parsing where "idx" is the index of the argument for a LAMMPS command - that is causing the failure (use -1 for the command itself). The - output will then not only be the last input line, but also the command - and arguments *after* they have been pre-processed and split into - individual arguments and a textual indicator pointing to the specific - word that failed. + parsing where "idx" is the index (starting at 0) of the argument for a + LAMMPS command that is causing the failure (use -1 for the command + itself). The output may also include the last input line *before* and + *after*, if they differ due to substituting variables. A textual + indicator is pointing to the specific word that failed. Using the + constant ``Error::NOPOINTER`` in place of the *idx* argument will + suppress the marker and then the behavior is like the *idx* argument + is not provided. FLERR is a macro containing the filename and line where the Error class is called and that information is appended to the error message. This @@ -285,6 +287,13 @@ the arguments are then handed for formatting to the `{fmt} library `_ (which is bundled with LAMMPS) and thus allow processing similar to the "format()" functionality in Python. +.. note:: + + For commands like :doc:`fix ave/time ` that accept + wildcard arguments, the :cpp:func:`utils::expand_args` function + may be passed as an optional argument where the function will provide + a map to the original arguments from the expanded argument indices. + For complex errors, that can have multiple causes and which cannot be explained in a single line, you can append to the error message, the string created by :cpp:func:`utils::errorurl`, which then provides a diff --git a/doc/src/Run_output.rst b/doc/src/Run_output.rst index d79882511a..28ed891765 100644 --- a/doc/src/Run_output.rst +++ b/doc/src/Run_output.rst @@ -203,26 +203,39 @@ Two lines In addition to the single line output, also the last line of the input will be repeated. If a command is spread over multiple lines in the input using the continuation character '&', then the error will print -the entire concatenated line. However, there is no further processing -of that line. Example: +the entire concatenated line. For readability all whitespace is +compressed to single blanks. Example: .. parsed-literal:: ERROR: Unrecognized fix style 'printf' (src/modify.cpp:924) - Last input line: fix 0 all printf v_nevery 'Step: $(step) ${step}' + Last input line: fix 0 all printf v_nevery "Step: $(step) ${step}" -Four lines -^^^^^^^^^^ +Three lines +^^^^^^^^^^^ -In addition to the two line output, the last command is printed a second -time, but *after* all variables were substituted and the line separated -into "words" as they are passed internally to the called command. This line -is followed by a line with caret character markers '^' to indicate which -"word" in the parsed input is causing the failure. Example: +In addition to the two line output from above, a third line is added +that uses caret character markers '^' to indicate which "word" in the +input failed. Example: .. parsed-literal:: ERROR: Illegal fix print nevery value -100; must be > 0 (src/fix_print.cpp:41) - Last input line: fix 0 all print ${nevery} 'Step: $(step) ${step}' + Last input line: fix 0 all print -100 "Step: $(step) ${stepx}" + ^^^^ + +Four lines +^^^^^^^^^^ + +The three line output is expanded to four lines, if the the input is +modified through input pre-processing, e.g. when substituting +variables. Now the last command is printed once in the original form and +a second time after substitutions are applied. The caret character +markers '^' are applied to the second version. Example: + +.. parsed-literal:: + + ERROR: Illegal fix print nevery value -100; must be > 0 (src/fix_print.cpp:41) + Last input line: fix 0 all print ${nevery} 'Step: $(step) ${step}' --> parsed line: fix 0 all print -100 "Step: $(step) ${step}" ^^^^ diff --git a/src/utils.h b/src/utils.h index 21300c4943..38f91cfc66 100644 --- a/src/utils.h +++ b/src/utils.h @@ -402,9 +402,9 @@ This functions adds the following case to :cpp:func:`utils::bounds() Date: Mon, 20 Jan 2025 14:57:11 -0500 Subject: [PATCH 41/41] use ioffset variable to indicate the first argument that is not fixed also, convert all error messages to use the new style. --- src/fix_ave_time.cpp | 101 +++++++++++++++++++++++++------------------ 1 file changed, 59 insertions(+), 42 deletions(-) diff --git a/src/fix_ave_time.cpp b/src/fix_ave_time.cpp index 3ac8e68cad..b48b5cd7ba 100644 --- a/src/fix_ave_time.cpp +++ b/src/fix_ave_time.cpp @@ -60,7 +60,9 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : // then read options so know mode = SCALAR/VECTOR before re-reading values nvalues = 0; - int iarg = 6; + // the first six arguments have fixed positions + const int ioffset = 6; + int iarg = ioffset; while (iarg < narg) { if (utils::strmatch(arg[iarg],"^[cfv]_")) { nvalues++; @@ -68,9 +70,10 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : } else break; } if (nvalues == 0) - error->all(FLERR, 6, "No values from computes, fixes, or variables used in fix ave/time command"); + error->all(FLERR, ioffset, + "No values from computes, fixes, or variables used in fix ave/time command"); - // parse optional keywords + // parse optional keywords which must follow the data options(iarg,narg,arg); @@ -80,10 +83,10 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : int expand = 0; char **earg; int *amap = nullptr; - nvalues = utils::expand_args(FLERR,nvalues,&arg[6],mode,earg,lmp,&amap); + nvalues = utils::expand_args(FLERR,nvalues,&arg[ioffset],mode,earg,lmp,&amap); key2col.clear(); - if (earg != &arg[6]) expand = 1; + if (earg != &arg[ioffset]) expand = 1; arg = earg; // parse values @@ -98,11 +101,11 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : key2col[arg[i]] = i; if ((val.which == ArgInfo::NONE) || (val.which == ArgInfo::UNKNOWN) || (argi.get_dim() > 1)) - error->all(FLERR, amap[i]+6,"Invalid fix ave/time argument: {}", arg[i]); + error->all(FLERR, amap[i] + ioffset,"Invalid fix ave/time argument: {}", arg[i]); val.argindex = argi.get_index1(); - if (expand) val.iarg = amap[i] + 6; - else val.iarg = i + 6; + if (expand) val.iarg = amap[i] + ioffset; + else val.iarg = i + ioffset; val.varlen = 0; val.offcol = 0; val.id = argi.get_name(); @@ -118,7 +121,7 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : for (int i = 0; i < noff; i++) { if (offlist[i] < 1 || offlist[i] > nvalues) error->all(FLERR,"Invalid fix ave/time off column: {}", offlist[i]); - values[offlist[i]-1].offcol = 1; + values[offlist[i] - 1].offcol = 1; } // setup and error check @@ -167,47 +170,54 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : val.val.f = modify->get_fix_by_id(val.id); if (!val.val.f) error->all(FLERR,"Fix ID {} for fix ave/time does not exist", val.id); if ((val.argindex == 0) && (val.val.f->scalar_flag == 0)) - error->all(FLERR,"Fix ave/time fix {} does not calculate a scalar", val.id); + error->all(FLERR, val.iarg, "Fix ave/time fix {} does not calculate a scalar", val.id); if (val.argindex && (val.val.f->vector_flag == 0)) - error->all(FLERR,"Fix ave/time fix {} does not calculate a vector", val.id); + error->all(FLERR, val.iarg, "Fix ave/time fix {} does not calculate a vector", val.id); if (val.argindex && (val.val.f->size_vector_variable)) - error->all(FLERR,"Fix ave/time fix {} vector cannot be variable length", val.id); + error->all(FLERR, val.iarg, "Fix ave/time fix {} vector cannot be variable length", val.id); if (val.argindex && (val.argindex > val.val.f->size_vector)) - error->all(FLERR,"Fix ave/time fix {} vector is accessed out-of-range", val.id); + error->all(FLERR, val.iarg, "Fix ave/time fix {} vector is accessed out-of-range", val.id); if (nevery % val.val.f->global_freq) - error->all(FLERR, "Fix {} for fix ave/time not computed at compatible time", val.id); + error->all(FLERR, val.iarg, "Fix {} for fix ave/time not computed at compatible time", + val.id); } else if ((val.which == ArgInfo::FIX) && (mode == VECTOR)) { val.val.f = modify->get_fix_by_id(val.id); - if (!val.val.f) error->all(FLERR,"Fix ID {} for fix ave/time does not exist", val.id); + if (!val.val.f) + error->all(FLERR, val.iarg, "Fix ID {} for fix ave/time does not exist", val.id); if ((val.argindex == 0) && (val.val.f->vector_flag == 0)) - error->all(FLERR,"Fix ave/time fix {} does not calculate a vector", val.id); + error->all(FLERR, val.iarg, "Fix ave/time fix {} does not calculate a vector", val.id); if (val.argindex && (val.val.f->array_flag == 0)) - error->all(FLERR,"Fix ave/time fix {} does not calculate an array", val.id); + error->all(FLERR, val.iarg, "Fix ave/time fix {} does not calculate an array", val.id); if (val.argindex && (val.val.f->size_array_rows_variable)) - error->all(FLERR,"Fix ave/time fix {} array cannot have variable row length", val.id); + error->all(FLERR, val.iarg, "Fix ave/time fix {} array cannot have variable row length", + val.id); if (val.argindex && (val.argindex > val.val.f->size_array_cols)) - error->all(FLERR,"Fix ave/time fix {} array is accessed out-of-range", val.id); + error->all(FLERR, val.iarg, "Fix ave/time fix {} array is accessed out-of-range", val.id); if (nevery % val.val.f->global_freq) - error->all(FLERR, "Fix {} for fix ave/time not computed at compatible time", val.id); + error->all(FLERR, val.iarg, "Fix {} for fix ave/time not computed at compatible time", + val.id); } else if ((val.which == ArgInfo::VARIABLE) && (mode == SCALAR)) { int ivariable = input->variable->find(val.id.c_str()); if (ivariable < 0) - error->all(FLERR,"Variable name {} for fix ave/time does not exist", val.id); + error->all(FLERR, val.iarg, "Variable name {} for fix ave/time does not exist", val.id); if ((val.argindex == 0) && (input->variable->equalstyle(ivariable) == 0)) - error->all(FLERR,"Fix ave/time variable {} is not equal-style variable", val.id); + error->all(FLERR, val.iarg, "Fix ave/time variable {} is not equal-style variable", val.id); if ((val.argindex) && (input->variable->vectorstyle(ivariable) == 0)) - error->all(FLERR,"Fix ave/time variable {} is not vector-style variable", val.id); + error->all(FLERR, val.iarg, "Fix ave/time variable {} is not vector-style variable", + val.id); } else if ((val.which == ArgInfo::VARIABLE) && (mode == VECTOR)) { int ivariable = input->variable->find(val.id.c_str()); if (ivariable < 0) - error->all(FLERR,"Variable name {} for fix ave/time does not exist", val.id); + error->all(FLERR, val.iarg, "Variable name {} for fix ave/time does not exist", val.id); if ((val.argindex == 0) && (input->variable->vectorstyle(ivariable) == 0)) - error->all(FLERR,"Fix ave/time variable {} is not vector-style variable", val.id); + error->all(FLERR, val.iarg, "Fix ave/time variable {} is not vector-style variable", + val.id); if (val.argindex) - error->all(FLERR,"Fix ave/time mode vector variable {} cannot be indexed", val.id); + error->all(FLERR, val.iarg, "Fix ave/time mode vector variable {} cannot be indexed", + val.id); val.varlen = 1; } } @@ -265,7 +275,9 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : fprintf(fp,"\n"); } if (yaml_flag) fputs("---\n",fp); - if (ferror(fp)) error->one(FLERR,"Error writing file header: {}", utils::getsyserror()); + if (ferror(fp)) + error->one(FLERR, Error::NOLASTLINE, "Error writing fix ave/time ID {} file header: {}", + id, utils::getsyserror()); filepos = platform::ftell(fp); } @@ -385,12 +397,12 @@ FixAveTime::FixAveTime(LAMMPS *lmp, int narg, char **arg) : extvalue = 0; } if (extvalue == -1) - error->all(FLERR,"Fix ave/time cannot set output array intensive/extensive " - "from these inputs"); + error->all(FLERR, Error::NOLASTLINE, "Fix ave/time cannot set output array " + "intensive/extensive from these inputs"); if (extarray < -1) extarray = extvalue; else if (extvalue != extarray) - error->all(FLERR,"Fix ave/time cannot set output array intensive/extensive " - "from these inputs"); + error->all(FLERR, Error::NOLASTLINE, "Fix ave/time cannot set output array " + "intensive/extensive from these inputs"); } } } @@ -470,15 +482,17 @@ void FixAveTime::init() if (val.which == ArgInfo::COMPUTE) { val.val.c = modify->get_compute_by_id(val.id); if (!val.val.c) - error->all(FLERR,"Compute ID {} for fix ave/time does not exist", val.id); + error->all(FLERR, Error::NOLASTLINE, "Compute ID {} for fix ave/time does not exist", + val.id); } else if (val.which == ArgInfo::FIX) { val.val.f = modify->get_fix_by_id(val.id); if (!val.val.f) - error->all(FLERR,"Fix ID {} for fix ave/time does not exist", val.id); + error->all(FLERR, Error::NOLASTLINE, "Fix ID {} for fix ave/time does not exist", val.id); } else if (val.which == ArgInfo::VARIABLE) { val.val.v = input->variable->find(val.id.c_str()); if (val.val.v < 0) - error->all(FLERR,"Variable name {} for fix ave/time does not exist", val.id); + error->all(FLERR, Error::NOLASTLINE, "Variable name {} for fix ave/time does not exist", + val.id); } } @@ -663,14 +677,16 @@ void FixAveTime::invoke_scalar(bigint ntimestep) fmt::print(fp,"{}",ntimestep); for (i = 0; i < nvalues; i++) fprintf(fp,format,vector_total[i]/norm); fprintf(fp,"\n"); - if (ferror(fp)) error->one(FLERR,"Error writing out time averaged data"); + if (ferror(fp)) + error->one(FLERR, Error::NOLASTLINE, "Error writing out time averaged data: {}", + utils::getsyserror()); } fflush(fp); if (overwrite) { bigint fileend = platform::ftell(fp); if ((fileend > 0) && (platform::ftruncate(fp,fileend))) - error->warning(FLERR,"Error while tuncating output: {}", utils::getsyserror()); + error->warning(FLERR, "Error while tuncating output: {}", utils::getsyserror()); } } } @@ -775,7 +791,8 @@ void FixAveTime::invoke_vector(bigint ntimestep) double *varvec; int nvec = input->variable->compute_vector(val.val.v,&varvec); if (nvec != nrows) - error->all(FLERR,"Fix ave/time vector-style variable {} changed length", val.id); + error->all(FLERR, Error::NOLASTLINE, "Fix ave/time vector-style variable {} changed length", + val.id); for (int i = 0; i < nrows; i++) column[i] = varvec[i]; } @@ -889,7 +906,7 @@ void FixAveTime::invoke_vector(bigint ntimestep) if (overwrite) { bigint fileend = platform::ftell(fp); if ((fileend > 0) && (platform::ftruncate(fp,fileend))) - error->warning(FLERR,"Error while tuncating output: {}", utils::getsyserror()); + error->warning(FLERR, "Error while tuncating output: {}", utils::getsyserror()); } } } @@ -920,7 +937,7 @@ int FixAveTime::column_length(int dynamic) } if (length == 0) length = lengthone; else if (lengthone != length) - error->all(FLERR,"Fix ave/time columns are inconsistent lengths"); + error->all(FLERR, Error::NOLASTLINE, "Fix ave/time columns have inconsistent lengths"); } } @@ -943,10 +960,10 @@ int FixAveTime::column_length(int dynamic) if (all_variable_length) { if (length == 0) length = lengthone; else if (lengthone != length) - error->all(FLERR,"Fix ave/time columns are inconsistent lengths"); + error->all(FLERR, Error::NOLASTLINE, "Fix ave/time columns have inconsistent lengths"); } else { if (lengthone != nrows) - error->all(FLERR,"Fix ave/time columns are inconsistent lengths"); + error->all(FLERR, Error::NOLASTLINE, "Fix ave/time columns have inconsistent lengths"); } } } @@ -1010,7 +1027,7 @@ int FixAveTime::modify_param(int narg, char **arg) } } if ((icol < 0) || (icol >= (int) values.size())) - error->all(FLERR, "Thermo_modify colname column {} invalid", arg[1]); + error->all(FLERR, 1 + 1, "Thermo_modify colname column {} invalid", arg[1]); values[icol].keyword = arg[2]; return 3; }