Parallel Ising example: operator precedence makes the heat-bath energy difference wrong

Open Beginner friendly
  • Is something wrong?
#313 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
cmake, cpp
Domain
distributed-systems, testing

Research direction

Start in tutorials/examples/parapack/multiple/ising.h at line 98 and compare the spin-flip energy difference with the correct energy calculation on lines 106–109. Fix the operator grouping, then check the example’s energy per site against tanh(J/T) across rank counts. The issue also asks to regenerate and register the 2- and 4-rank MPI reference tests, or remove the unused reference files; inspect multiple/CMakeLists.txt first.

Written by the indexing model from the issue text.

Description

area:tutorials bug scientific-correctness

Summary

tutorials/examples/parapack/multiple/ising.h:98 computes the energy change of a spin flip as

double diff = coupling_ * (4 * (spins_[i-1] ^ spins_[i] + spins_[i] ^ spins_[i+1]) - 4);

+ binds tighter than ^, so this is 4 * (l ^ (c + c) ^ r) - 4, not
4 * ((l ^ c) + (c ^ r)) - 4. For the eight neighbour configurations the two agree in only
four:

l c r | as written | intended
0 0 0 |  -4 |  -4
0 0 1 |   0 |   0
0 1 0 |   4 |   4
0 1 1 |   8 |   0
1 0 0 |   0 |   0
1 0 1 |  -4 |   4
1 1 0 |   8 |   0
1 1 1 |   4 |  -4

The acceptance 0.5 * (1 + tanh(-0.5 * beta * diff)) on line 99 therefore samples the wrong
ensemble (g++ -Wparentheses flags the line). The energy measured on lines 106-109 uses a
correct expression, so the reported energies are simply those of the wrong dynamics: for the
L = 16 chain at T = 2 the example reports Energy = 9.6 ± 0.06 at 1, 2, 4 and 8 ranks,
while the exact value of the measured Hamiltonian is 16 tanh(0.5) = 7.39 in magnitude.
The line dates from 2008 (badd508f6b) and is unchanged by #284.

The sibling single-process examples (exchange/ising.C, ising/ising.C) use a different
expression and are not affected. #184 fixed a different sign error in the Python Ising
tutorials.

Related: the reference outputs are stale and unused

multiple/ising.op-2 and ising.op-4 (2009) expect a [results] block with
Temperature/Inverse Temperature observables. The current --mpi stdin driver prints
[results 0], [results 1], ... per rank and the SimpleRealObservables do not appear, so
the files could never match, and no test registers them (multiple/CMakeLists.txt adds only
the executable).

Fix

double diff = coupling_ * (4 * ((spins_[i-1] ^ spins_[i]) + (spins_[i] ^ spins_[i+1])) - 4);

Then either regenerate ising.op-2/ising.op-4 from the fixed example and register them as
2- and 4-rank MPI tests, or delete them. A cheap correctness check for the test: with the fix,
Energy / Number of Sites must agree with tanh(J/T) within error at every rank count.

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.