exchange_multi.h distributes replicas over all ranks instead of over process groups

Open Beginner friendly
  • Is something wrong?
#312 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 3 days

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp
Domain
distributed-systems, testing

Research direction

Read src/alps/parapack/include/alps/parapack/exchange_multi.h around calc_nrep at lines 435–445, then trace its callers at lines 90 and 104 to confirm the rank group used. Replace the three total-communicator size references as described, and add the supplied input as an MPI tutorial test. Done means the test reports 2 2 replicas per process group with four replicas and two groups.

Written by the indexing model from the issue text.

Description

area:parapack bug scientific-correctness

Summary

multiple_parallel_exchange_worker (replica exchange where each replica is itself a
parallel worker over PROCESS_PER_WORKER ranks) computes the number of replicas per group
by dividing by the total rank count. With PROCESS_PER_WORKER > 1 and more replicas than
groups, only #groups walkers are created while the temperature ladder keeps all
NUM_REPLICAS entries. The exchange step then uses weights of walkers that do not exist.

Reproducer

tutorials/examples/parapack/exchange built against an MPI SDK, input

ALGORITHM = "multiple parallel ising; exchange"
L = 16
SEED = 2873
SWEEPS = 1024
NUM_REPLICAS = 4
PROCESS_PER_WORKER = 2
T_MIN = 2
T_MAX = 4
TEMPERATURE_DISTRIBUTION_TYPE = 2
OPTIMIZE_TEMPERATURE = 1
{ RANDOM_EXCHANGE = 1 }

mpirun -np 4 exchange --mpi < input prints

EXMC: number of replicas = 4
EXMC: number of replicas on each process = 1 1
EXMC: initial inverse temperature set = 0.25 0.3 0.375 0.5
EXMC stage 0: acceptance rate = 0.91406 0.86719 0.86719

Two groups, four temperatures, two walkers. The run completes and reports acceptance rates
for three pairs as if four replicas existed. NUM_REPLICAS = 2 with the same layout happens
to be right (0 + 1 per group by the remainder rule), which is why nothing noticed.

Location

src/alps/parapack/include/alps/parapack/exchange_multi.h:435-445:

std::pair<int, int> calc_nrep(int id) const {
  int nrep = beta_.size();
  int n = nrep / comm_.size();          // comm_ = all ranks; should be head_.size()
  ...
  if (id < nrep - n * comm_.size()) {   // same
  ...
    f = (nrep - n * comm_.size()) + n * id;

id is a rank in head_ (one head per group): line 90 calls calc_nrep(head_.rank())
and line 104 loops for (p = 0; p < head_.size(); ++p) calc_nrep(p). The function was copied
from exchange.h:1198, where comm_ is the communicator the replicas live on, and never
adjusted. The code is from 2010 and unchanged by #284, which only converted the send/recv
calls in this header.

Consequences

tid_, wp_ and weight_parameters_ are sized nrep (lines 126, 131, 151), but only
sum(nreps_) entries of wp_ are ever filled (line 216). The exchange acceptance at lines
235-256 and the temperature optimisation at line 297 read the unfilled entries (zero after
resize), so the ladder optimisation and the exchange statistics are wrong whenever
NUM_REPLICAS > comm_.size() / PROCESS_PER_WORKER.

Fix

Replace comm_.size() by head_.size() in calc_nrep (three places). head_ is
MPI_COMM_NULL on non-head ranks, but calc_nrep is only called inside if (head_) blocks.
Add the input above as an MPI test of the exchange tutorial (see the CI issue on parapack
MPI tutorials); with the fix it must print 2 2.

Dominant language
C++
Stars
32
Forks
10
Avg merge
4d 6h
Merged PRs (30d)
51

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ALPSim/ALPS

All issues in ALPSim/ALPS

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.