Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mpi walker transmission fix #2232

Merged
merged 13 commits into from
Jan 24, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions CMake/ClangCompilers.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ ENDIF(QMC_OMP)
# Set clang specific flags (which we always want)
ADD_DEFINITIONS( -Drestrict=__restrict__ )

SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fomit-frame-pointer -fstrict-aliasing")
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fomit-frame-pointer -fstrict-aliasing -D__forceinline=inline")
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this causing problems in debug build?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it appears that the first -fomit -fno-omit wins.

Copy link
Contributor

@ye-luo ye-luo Jan 23, 2020

Choose a reason for hiding this comment

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

CMAKE_XXX_FLAGS is added to all builds.
For a build release CMake invokes both $CMAKE_XXX_FLAGS $CMAKE_XXX_FLAGS_RELEASE. If all the builds are intended, add to CMAKE_XXX_FLAGS. If release builds are intended, add to CMAKE_XXX_FLAGS_RELEASE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean the CMAKE_C(XX)_FLAGS should not contain -f-omit-frame-pointer if you don't want that in the flags for the debug build. This fixes that. While leaving this optimization in the default RelWithDebugInfo and Release.

SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fstrict-aliasing")
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fstrict-aliasing -D__forceinline=inline")

# Set extra optimization specific flags
SET( CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -ffast-math" )
SET( CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -ffast-math" )
SET( CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} -ffast-math" )
SET( CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} -ffast-math" )
SET( CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -fomit-frame-pointer -ffast-math" )
SET( CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -fomit-frame-pointer -ffast-math" )
SET( CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} -fomit-frame-pointer -ffast-math" )
SET( CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} -fomit-frame-pointer -ffast-math" )

# Set extra debug flags
SET( CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -fno-omit-frame-pointer -fstandalone-debug" )
Expand Down
15 changes: 9 additions & 6 deletions CMake/GNUCompilers.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ ENDIF(QMC_OMP)
# Set gnu specific flags (which we always want)
ADD_DEFINITIONS( -Drestrict=__restrict__ )

SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fomit-frame-pointer -finline-limit=1000 -fstrict-aliasing -funroll-all-loops")
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fomit-frame-pointer -finline-limit=1000 -fstrict-aliasing -funroll-all-loops -D__forceinline=inline")
SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -finline-limit=1000 -fstrict-aliasing -funroll-all-loops")
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -finline-limit=1000 -fstrict-aliasing -funroll-all-loops -D__forceinline=inline")

SET( CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -fno-omit-frame-pointer" )
SET( CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fno-omit-frame-pointer" )

# Suppress compile warnings
SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-deprecated")
Expand All @@ -32,10 +35,10 @@ SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}" )
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wcomment -Wmisleading-indentation -Wmaybe-uninitialized -Wuninitialized -Wreorder -Wno-unknown-pragmas -Wno-sign-compare")

# Set extra optimization specific flags
SET( CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -ffast-math" )
SET( CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -ffast-math" )
SET( CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} -ffast-math" )
SET( CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} -ffast-math" )
SET( CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} -fomit-frame-pointer -ffast-math" )
SET( CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -fomit-frame-pointer -ffast-math" )
SET( CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} -fomit-frame-pointer -ffast-math" )
SET( CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} -fomit-frame-pointer -ffast-math" )

# Special architectural flags
#--------------------------------------
Expand Down
2 changes: 0 additions & 2 deletions src/MinimalContainers/ConstantSizeMatrix.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,6 @@ class ConstantSizeMatrix
T* data() { return data_.data(); }
const T* data() const { return data_.data(); }

size_t sizeofElement() { return sizeof(T); }

size_t capacity() { return n_max_ * m_max_; }
size_t n_capacity() { return n_max_; }

Expand Down
9 changes: 9 additions & 0 deletions src/Particle/MCWalkerConfiguration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,15 @@ void MCWalkerConfiguration::copyWalkerRefs(Walker_t* head, Walker_t* tail)
WalkerList[1] = tail;
}

void MCWalkerConfiguration::fakeWalkerList(Walker_t* first, Walker_t* second)
{
if (WalkerList.size() != 0)
throw std::runtime_error("This should only be called in tests and only with a fresh MCWC!");
OwnWalkers = false;
WalkerList.push_back(first);
WalkerList.push_back(second);
}

/** Make Metropolis move to the walkers and save in a temporary array.
* @param it the iterator of the first walker to work on
* @param tauinv inverse of the time step
Expand Down
5 changes: 5 additions & 0 deletions src/Particle/MCWalkerConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,17 @@ class MCWalkerConfiguration : public ParticleSet
* @param head pointer to the head walker
* @param tail pointer to the tail walker
*
* \todo I believe this can/should be deleted
* Special function introduced to work with Reptation method.
* Clear the current WalkerList and add two walkers, head and tail.
* OwnWalkers are set to false.
*/
void copyWalkerRefs(Walker_t* head, Walker_t* tail);

/** make fake walker list for testing
*/
void fakeWalkerList(Walker_t* first, Walker_t* second);

///clean up the walker list and make a new list
void resize(int numWalkers, int numPtcls);

Expand Down
6 changes: 3 additions & 3 deletions src/Particle/Walker.h
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ struct Walker
#endif
//Don't add the nLocal but the actual allocated size. We want to register once for the life of a
//walker so we leave space for additional properties.
DataSet.add(Properties.data(), Properties.data() + Properties.sizeofElement() * Properties.capacity());
DataSet.add(Properties.data(), Properties.data() + Properties.capacity());
//DataSet.add(Properties.first_address(), Properties.last_address());

// \todo likely to be broken if the Properties change above is needed.
Expand Down Expand Up @@ -463,7 +463,7 @@ struct Walker
DataSet.get(G.first_address(), G.last_address());
DataSet.get(L.first_address(), L.last_address());
#endif
DataSet.get(Properties.data(), Properties.data() + Properties.sizeofElement() * Properties.capacity());
DataSet.get(Properties.data(), Properties.data() + Properties.capacity());
for (int iat = 0; iat < PropertyHistory.size(); iat++)
DataSet.get(PropertyHistory[iat].data(), PropertyHistory[iat].data() + PropertyHistory[iat].size());
DataSet.get(PHindex.data(), PHindex.data() + PHindex.size());
Expand Down Expand Up @@ -512,7 +512,7 @@ struct Walker
DataSet.put(G.first_address(), G.last_address());
DataSet.put(L.first_address(), L.last_address());
#endif
DataSet.put(Properties.data(), Properties.data() + Properties.sizeofElement() * Properties.capacity());
DataSet.put(Properties.data(), Properties.data() + Properties.capacity());
for (int iat = 0; iat < PropertyHistory.size(); iat++)
DataSet.put(PropertyHistory[iat].data(), PropertyHistory[iat].data() + PropertyHistory[iat].size());
DataSet.put(PHindex.data(), PHindex.data() + PHindex.size());
Expand Down
43 changes: 36 additions & 7 deletions src/Particle/tests/test_walker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@
// This file is distributed under the University of Illinois/NCSA Open Source License.
// See LICENSE file in top directory for details.
//
// Copyright (c) 2016 Jeongnim Kim and QMCPACK developers.
// Copyright (c) 2020 QMCPACK developers.
//
// File developed by: Mark Dewing, markdewing@gmail.com, University of Illinois at Urbana-Champaign
// File developed by: Peter Doak, doakpw@ornl.gov, Oak Ridge National Lab
// Mark Dewing, markdewing@gmail.com, University of Illinois at Urbana-Champaign
//
// File created by: Mark Dewing, markdewing@gmail.com, University of Illinois at Urbana-Champaign
//////////////////////////////////////////////////////////////////////////////////////

#include "catch.hpp"

#include <cstring>
#include "QMCDrivers/WalkerProperties.h"
#include "Configuration.h"
#include "Particle/MCWalkerConfiguration.h"
#include "Particle/HDFWalkerOutput.h"
Expand All @@ -24,13 +27,15 @@ using std::string;

namespace qmcplusplus
{
typedef Walker<QMCTraits, PtclOnLatticeTraits> Walker_t;

using MCPWalker = Walker<QMCTraits, PtclOnLatticeTraits>;
using WP = WalkerProperties::Indexes;

TEST_CASE("walker", "[particle]")
{
OHMMS::Controller->initialize(0, NULL);

Walker_t w(1);
MCPWalker w(1);
REQUIRE(w.R.size() == 1);
w.R[0] = 1.0;

Expand All @@ -44,7 +49,7 @@ TEST_CASE("walker", "[particle]")
TEST_CASE("walker assumptions", "[particle]")
{
using WP = WalkerProperties::Indexes;
Walker_t w1(1);
MCPWalker w1(1);
REQUIRE(w1.Properties.cols() == WP::NUMPROPERTIES);
}

Expand All @@ -53,10 +58,10 @@ TEST_CASE("walker HDF read and write", "[particle]")
OHMMS::Controller->initialize(0, NULL);
Communicate* c = OHMMS::Controller;

Walker_t w1(1);
MCPWalker w1(1);
w1.R[0] = 1.0;

Walker_t w2(1);
MCPWalker w2(1);
w2.R[0] = 0.5;

MCWalkerConfiguration W;
Expand Down Expand Up @@ -110,4 +115,28 @@ TEST_CASE("walker HDF read and write", "[particle]")
}
}

TEST_CASE("walker buffer add, update, restore", "[particle]")
{
int num_particles = 4;

UPtrVector<MCPWalker> walkers(2);
auto createWalker = [num_particles](UPtr<MCPWalker>& walker_ptr) {
walker_ptr = std::make_unique<MCPWalker>(num_particles);
walker_ptr->registerData();
walker_ptr->DataSet.allocate();
};
std::for_each(walkers.begin(), walkers.end(), createWalker);

walkers[0]->Properties(WP::LOGPSI) = 1.2;
walkers[0]->Properties(WP::SIGN) = 1.3;
walkers[0]->Properties(WP::UMBRELLAWEIGHT) = 1.4;
walkers[0]->Properties(WP::LOCALPOTENTIAL) = 1.6;
walkers[0]->updateBuffer();

std::memcpy(walkers[1]->DataSet.data(), walkers[0]->DataSet.data(), walkers[0]->DataSet.size());
walkers[1]->copyFromBuffer();
CHECK(walkers[1]->Properties(WP::LOGPSI) == Approx(1.2));
CHECK(walkers[1]->Properties(WP::LOCALPOTENTIAL) == Approx(1.6));
}

} // namespace qmcplusplus
2 changes: 1 addition & 1 deletion src/QMCApp/QMCMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ bool QMCMain::runQMC(xmlNodePtr cur, bool reuse)

if(!population_)
{
population_.reset(new MCPopulation(myComm->size(), *qmcSystem, ptclPool->getParticleSet("e"), psiPool->getPrimary(), hamPool->getPrimary()));
population_.reset(new MCPopulation(myComm->size(), *qmcSystem, ptclPool->getParticleSet("e"), psiPool->getPrimary(), hamPool->getPrimary(), myComm->rank()));
}
if (reuse)
qmc_driver = std::move(last_driver);
Expand Down
8 changes: 4 additions & 4 deletions src/QMCApp/tests/test_QMCDriverFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ TEST_CASE("QMCDriverFactory create VMC Driver", "[qmcapp]")
std::unique_ptr<QMCDriverInterface> qmc_driver;

MCPopulation population(comm->size(), particle_pool.getParticleSet("e"), wavefunction_pool.getPrimary(),
hamiltonian_pool.getPrimary());
hamiltonian_pool.getPrimary(), comm->rank());

qmc_driver = driver_factory.newQMCDriver(std::move(last_driver), 0, node, das, *qmc_system, particle_pool,
wavefunction_pool, hamiltonian_pool, population, comm);
Expand Down Expand Up @@ -96,7 +96,7 @@ TEST_CASE("QMCDriverFactory create VMCBatched driver", "[qmcapp]")
std::unique_ptr<QMCDriverInterface> last_driver;
std::unique_ptr<QMCDriverInterface> qmc_driver;
MCPopulation population(comm->size(), particle_pool.getParticleSet("e"), wavefunction_pool.getPrimary(),
hamiltonian_pool.getPrimary());
hamiltonian_pool.getPrimary(), comm->rank());

qmc_driver = driver_factory.newQMCDriver(std::move(last_driver), 0, node, das, *qmc_system, particle_pool,
wavefunction_pool, hamiltonian_pool, population, comm);
Expand Down Expand Up @@ -132,7 +132,7 @@ TEST_CASE("QMCDriverFactory create DMC driver", "[qmcapp]")
std::unique_ptr<QMCDriverInterface> last_driver;
std::unique_ptr<QMCDriverInterface> qmc_driver;
MCPopulation population(comm->size(), particle_pool.getParticleSet("e"), wavefunction_pool.getPrimary(),
hamiltonian_pool.getPrimary());
hamiltonian_pool.getPrimary(),comm->rank());

qmc_driver = driver_factory.newQMCDriver(std::move(last_driver), 0, node, das, *qmc_system, particle_pool,
wavefunction_pool, hamiltonian_pool, population, comm);
Expand Down Expand Up @@ -168,7 +168,7 @@ TEST_CASE("QMCDriverFactory create DMCBatched driver", "[qmcapp]")
std::unique_ptr<QMCDriverInterface> last_driver;
std::unique_ptr<QMCDriverInterface> qmc_driver;
MCPopulation population(comm->size(), particle_pool.getParticleSet("e"), wavefunction_pool.getPrimary(),
hamiltonian_pool.getPrimary());
hamiltonian_pool.getPrimary(),comm->rank());

qmc_driver = driver_factory.newQMCDriver(std::move(last_driver), 0, node, das, *qmc_system, particle_pool,
wavefunction_pool, hamiltonian_pool, population, comm);
Expand Down
11 changes: 2 additions & 9 deletions src/QMCDrivers/Crowd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,8 @@ void Crowd::addWalker(MCPWalker& walker, ParticleSet& elecs, TrialWaveFunction&

void Crowd::loadWalkers()
{
auto it_walker = mcp_walkers_.begin();
auto it_walker_elecs = walker_elecs_.begin();
//flex walkers here
while (it_walker != mcp_walkers_.end())
{
(*it_walker_elecs).get().loadWalker(*it_walker, true);
++it_walker;
++it_walker_elecs;
}
for(int i = 0; i < mcp_walkers_.size(); ++i)
walker_elecs_[i].get().loadWalker(mcp_walkers_[i], true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Like this much better than iterators.

}

void Crowd::setRNGForHamiltonian(RandomGenerator_t& rng)
Expand Down
Loading