From: Mark Abraham Date: Fri, 20 Sep 2019 15:46:06 +0000 (+0200) Subject: Improve PME testing X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?a=commitdiff_plain;h=31c3bf677e89bbe262b58e06c6a1a82aa0209fb9;p=alexxy%2Fgromacs.git Improve PME testing The change to PME coordinate management uglified these tests even further. Creation semantics are now more clear. Change-Id: I309a487cdd218d85dbe4620e1d94e2f04f809a8d --- diff --git a/src/gromacs/ewald/tests/pmegathertest.cpp b/src/gromacs/ewald/tests/pmegathertest.cpp index 0d4e94db6c..d2afc1e324 100644 --- a/src/gromacs/ewald/tests/pmegathertest.cpp +++ b/src/gromacs/ewald/tests/pmegathertest.cpp @@ -388,13 +388,12 @@ class PmeGatherTest : public ::testing::TestWithParam TestReferenceData refData; for (const auto &context : getPmeTestEnv()->getHardwareContexts()) { - std::shared_ptr stateGpu; CodePath codePath = context->getCodePath(); const bool supportedInput = pmeSupportsInputForMode(*getPmeTestEnv()->hwinfo(), &inputRec, codePath); if (!supportedInput) { /* Testing the failure for the unsupported input */ - EXPECT_THROW(pmeInitAtoms(&inputRec, codePath, nullptr, nullptr, inputAtomData.coordinates, inputAtomData.charges, box, stateGpu), NotImplementedError); + EXPECT_THROW(pmeInitWrapper(&inputRec, codePath, nullptr, nullptr, box), NotImplementedError); continue; } @@ -408,8 +407,9 @@ class PmeGatherTest : public ::testing::TestWithParam (inputForceTreatment == PmeForceOutputHandling::ReduceWithInput) ? "with reduction" : "without reduction" )); - PmeSafePointer pmeSafe = pmeInitAtoms(&inputRec, codePath, context->getDeviceInfo(), - context->getPmeGpuProgram(), inputAtomData.coordinates, inputAtomData.charges, box, stateGpu); + PmeSafePointer pmeSafe = pmeInitWrapper(&inputRec, codePath, context->getDeviceInfo(), context->getPmeGpuProgram(), box); + StatePropagatorDataGpu stateGpu = makeStatePropagatorDataGpu(*pmeSafe.get()); + pmeInitAtoms(pmeSafe.get(), &stateGpu, codePath, inputAtomData.coordinates, inputAtomData.charges); /* Setting some more inputs */ pmeSetRealGrid(pmeSafe.get(), codePath, nonZeroGridValues); diff --git a/src/gromacs/ewald/tests/pmesplinespreadtest.cpp b/src/gromacs/ewald/tests/pmesplinespreadtest.cpp index a1c6eb4c1d..136c08b697 100644 --- a/src/gromacs/ewald/tests/pmesplinespreadtest.cpp +++ b/src/gromacs/ewald/tests/pmesplinespreadtest.cpp @@ -128,7 +128,7 @@ class PmeSplineAndSpreadTest : public ::testing::TestWithParamgetDeviceInfo(), - context->getPmeGpuProgram(), coordinates, charges, box, stateGpu); + PmeSafePointer pmeSafe = pmeInitWrapper(&inputRec, codePath, context->getDeviceInfo(), context->getPmeGpuProgram(), box); + StatePropagatorDataGpu stateGpu = makeStatePropagatorDataGpu(*pmeSafe.get()); + pmeInitAtoms(pmeSafe.get(), &stateGpu, codePath, coordinates, charges); const bool computeSplines = (option.first == PmeSplineAndSpreadOptions::SplineOnly) || (option.first == PmeSplineAndSpreadOptions::SplineAndSpreadUnified); const bool spreadCharges = (option.first == PmeSplineAndSpreadOptions::SpreadOnly) || (option.first == PmeSplineAndSpreadOptions::SplineAndSpreadUnified); diff --git a/src/gromacs/ewald/tests/pmetestcommon.cpp b/src/gromacs/ewald/tests/pmetestcommon.cpp index 61b28e587a..c3e538eb00 100644 --- a/src/gromacs/ewald/tests/pmetestcommon.cpp +++ b/src/gromacs/ewald/tests/pmetestcommon.cpp @@ -106,15 +106,14 @@ uint64_t getSplineModuliDoublePrecisionUlps(int splineOrder) return 4 * (splineOrder - 2) + 2 * sineUlps * splineOrder; } -//! PME initialization - internal -static PmeSafePointer pmeInitInternal(const t_inputrec *inputRec, - CodePath mode, - const gmx_device_info_t *gpuInfo, - PmeGpuProgramHandle pmeGpuProgram, - const Matrix3x3 &box, - real ewaldCoeff_q = 1.0F, - real ewaldCoeff_lj = 1.0F - ) +//! PME initialization +PmeSafePointer pmeInitWrapper(const t_inputrec *inputRec, + const CodePath mode, + const gmx_device_info_t *gpuInfo, + PmeGpuProgramHandle pmeGpuProgram, + const Matrix3x3 &box, + const real ewaldCoeff_q, + const real ewaldCoeff_lj) { const MDLogger dummyLogger; const auto runMode = (mode == CodePath::CPU) ? PmeRunMode::CPU : PmeRunMode::Mixed; @@ -156,7 +155,7 @@ static PmeSafePointer pmeInitInternal(const t_inputrec *inputRec, //! Simple PME initialization based on input, no atom data PmeSafePointer pmeInitEmpty(const t_inputrec *inputRec, - CodePath mode, + const CodePath mode, const gmx_device_info_t *gpuInfo, PmeGpuProgramHandle pmeGpuProgram, const Matrix3x3 &box, @@ -164,59 +163,58 @@ PmeSafePointer pmeInitEmpty(const t_inputrec *inputRec, real ewaldCoeff_lj ) { - return pmeInitInternal(inputRec, mode, gpuInfo, pmeGpuProgram, box, ewaldCoeff_q, ewaldCoeff_lj); + return pmeInitWrapper(inputRec, mode, gpuInfo, pmeGpuProgram, box, ewaldCoeff_q, ewaldCoeff_lj); // hiding the fact that PME actually needs to know the number of atoms in advance } +//! Make a GPU state-propagator manager +StatePropagatorDataGpu +makeStatePropagatorDataGpu(const gmx_pme_t &pme) +{ + // TODO: Pin the host buffer and use async memory copies + return StatePropagatorDataGpu(pme_gpu_get_device_stream(&pme), + pme_gpu_get_device_context(&pme), + GpuApiCallBehavior::Sync, + pme_gpu_get_padding_size(&pme)); +} + //! PME initialization with atom data -PmeSafePointer pmeInitAtoms(const t_inputrec *inputRec, - CodePath mode, - const gmx_device_info_t *gpuInfo, - PmeGpuProgramHandle pmeGpuProgram, - const CoordinatesVector &coordinates, - const ChargesVector &charges, - const Matrix3x3 &box, - std::shared_ptr stateGpu - ) +void pmeInitAtoms(gmx_pme_t *pme, + StatePropagatorDataGpu *stateGpu, + const CodePath mode, + const CoordinatesVector &coordinates, + const ChargesVector &charges) { - const index atomCount = coordinates.size(); + const index atomCount = coordinates.size(); GMX_RELEASE_ASSERT(atomCount == charges.ssize(), "Mismatch in atom data"); - PmeSafePointer pmeSafe = pmeInitInternal(inputRec, mode, gpuInfo, pmeGpuProgram, box); - PmeAtomComm *atc = nullptr; + PmeAtomComm *atc = nullptr; switch (mode) { case CodePath::CPU: - atc = &(pmeSafe->atc[0]); + atc = &(pme->atc[0]); atc->x = coordinates; atc->coefficient = charges; - gmx_pme_reinit_atoms(pmeSafe.get(), atomCount, charges.data()); + gmx_pme_reinit_atoms(pme, atomCount, charges.data()); /* With decomposition there would be more boilerplate atc code here, e.g. do_redist_pos_coeffs */ break; case CodePath::GPU: // TODO: Avoid use of atc in the GPU code path - atc = &(pmeSafe->atc[0]); + atc = &(pme->atc[0]); // We need to set atc->n for passing the size in the tests atc->setNumAtoms(atomCount); - gmx_pme_reinit_atoms(pmeSafe.get(), atomCount, charges.data()); + gmx_pme_reinit_atoms(pme, atomCount, charges.data()); - // TODO: Pin the host buffer and use async memory copies - stateGpu = std::make_unique(pme_gpu_get_device_stream(pmeSafe.get()), - pme_gpu_get_device_context(pmeSafe.get()), - GpuApiCallBehavior::Sync, - pme_gpu_get_padding_size(pmeSafe.get())); stateGpu->reinit(atomCount, atomCount); stateGpu->copyCoordinatesToGpu(arrayRefFromArray(coordinates.data(), coordinates.size()), gmx::StatePropagatorDataGpu::AtomLocality::All); - pme_gpu_set_kernelparam_coordinates(pmeSafe->gpu, stateGpu->getCoordinates()); + pme_gpu_set_kernelparam_coordinates(pme->gpu, stateGpu->getCoordinates()); break; default: GMX_THROW(InternalError("Test not implemented for this mode")); } - - return pmeSafe; } //! Getting local PME real grid pointer for test I/O diff --git a/src/gromacs/ewald/tests/pmetestcommon.h b/src/gromacs/ewald/tests/pmetestcommon.h index b51551c29a..dd27865ab7 100644 --- a/src/gromacs/ewald/tests/pmetestcommon.h +++ b/src/gromacs/ewald/tests/pmetestcommon.h @@ -118,6 +118,14 @@ uint64_t getSplineModuliDoublePrecisionUlps(int splineOrder); // PME stages +//! PME initialization +PmeSafePointer pmeInitWrapper(const t_inputrec *inputRec, + CodePath mode, + const gmx_device_info_t *gpuInfo, + PmeGpuProgramHandle pmeGpuProgram, + const Matrix3x3 &box, + real ewaldCoeff_q = 1.0F, + real ewaldCoeff_lj = 1.0F); //! Simple PME initialization (no atom data) PmeSafePointer pmeInitEmpty(const t_inputrec *inputRec, CodePath mode = CodePath::CPU, @@ -125,16 +133,15 @@ PmeSafePointer pmeInitEmpty(const t_inputrec *inputRec, PmeGpuProgramHandle pmeGpuProgram = nullptr, const Matrix3x3 &box = {{1.0F, 0.0F, 0.0F, 0.0F, 1.0F, 0.0F, 0.0F, 0.0F, 1.0F}}, real ewaldCoeff_q = 0.0F, real ewaldCoeff_lj = 0.0F); +//! Make a GPU state-propagator manager +StatePropagatorDataGpu +makeStatePropagatorDataGpu(const gmx_pme_t &pme); //! PME initialization with atom data and system box -PmeSafePointer pmeInitAtoms(const t_inputrec *inputRec, - CodePath mode, - const gmx_device_info_t *gpuInfo, - PmeGpuProgramHandle pmeGpuProgram, - const CoordinatesVector &coordinates, - const ChargesVector &charges, - const Matrix3x3 &box, - std::shared_ptr stateGpu - ); +void pmeInitAtoms(gmx_pme_t *pme, + StatePropagatorDataGpu *stateGpu, + CodePath mode, + const CoordinatesVector &coordinates, + const ChargesVector &charges); //! PME spline computation and charge spreading void pmePerformSplineAndSpread(gmx_pme_t *pme, CodePath mode, bool computeSplines, bool spreadCharges); diff --git a/src/gromacs/mdtypes/state_propagator_data_gpu.h b/src/gromacs/mdtypes/state_propagator_data_gpu.h index cf20737bce..a7cda77663 100644 --- a/src/gromacs/mdtypes/state_propagator_data_gpu.h +++ b/src/gromacs/mdtypes/state_propagator_data_gpu.h @@ -105,7 +105,11 @@ class StatePropagatorDataGpu const void *gpuContext, GpuApiCallBehavior transferKind, int paddingSize); - + //! Move constructor + StatePropagatorDataGpu(StatePropagatorDataGpu &&other) noexcept; + //! Move assignment + StatePropagatorDataGpu &operator=(StatePropagatorDataGpu &&other) noexcept; + //! Destructor ~StatePropagatorDataGpu(); /*! \brief Set the ranges for local and non-local atoms and reallocates buffers. @@ -217,7 +221,7 @@ class StatePropagatorDataGpu private: class Impl; gmx::PrivateImplPointer impl_; - + GMX_DISALLOW_COPY_AND_ASSIGN(StatePropagatorDataGpu); }; } // namespace gmx diff --git a/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp b/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp index dd0aa76f80..9a11956649 100644 --- a/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp +++ b/src/gromacs/mdtypes/state_propagator_data_gpu_impl.cpp @@ -62,9 +62,11 @@ StatePropagatorDataGpu::StatePropagatorDataGpu(const void * /* commandStre { } -StatePropagatorDataGpu::~StatePropagatorDataGpu() -{ -} +StatePropagatorDataGpu::StatePropagatorDataGpu(StatePropagatorDataGpu && /* other */) noexcept = default; + +StatePropagatorDataGpu &StatePropagatorDataGpu::operator=(StatePropagatorDataGpu && /* other */) noexcept = default; + +StatePropagatorDataGpu::~StatePropagatorDataGpu() = default; void StatePropagatorDataGpu::reinit(int /* numAtomsLocal */, int /* numAtomsAll */) diff --git a/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp b/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp index d2c1e5d54f..66cdfda9e9 100644 --- a/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp +++ b/src/gromacs/mdtypes/state_propagator_data_gpu_impl_gpu.cpp @@ -294,6 +294,10 @@ StatePropagatorDataGpu::StatePropagatorDataGpu(const void *commandStream, { } +StatePropagatorDataGpu::StatePropagatorDataGpu(StatePropagatorDataGpu && /* other */) noexcept = default; + +StatePropagatorDataGpu &StatePropagatorDataGpu::operator=(StatePropagatorDataGpu && /* other */) noexcept = default; + StatePropagatorDataGpu::~StatePropagatorDataGpu() = default;