Skip to content

Fix/review 2026 07 27 - #21

Closed
farrisric wants to merge 5 commits into
mainfrom
fix/review-2026-07-27
Closed

farrisric wants to merge 5 commits into
mainfrom
fix/review-2026-07-27

Conversation

@farrisric

Copy link
Copy Markdown
Owner

No description provided.

…findings

BatchedReplicaExchange._accept_swap implemented only the temperature-ladder
criterion (beta_j - beta_i)(Phi_j - Phi_i), which is identically zero when the
replicas share a temperature. Every mu-ladder swap was therefore accepted with
p = 1, so configurations random-walked freely across the ladder and no replica
sampled its own mu. Replace it with the general form

    beta_i Phi_X^(i) + beta_j Phi_Y^(j) - beta_i Phi_Y^(i) - beta_j Phi_X^(j)

where Phi_Z^(k) is a configuration scored with slot k's chemical potentials.
This reduces to the old expression for a shared mu, to beta (mu_i - mu_j)
(N_j - N_i) for a shared temperature, and also covers a joint (T, mu) ladder.
An EMT Ag(111)/Au ladder gives N_Au = [0, 0, 0, 14] with the fix and
[0, 0, 0, 0] without it: unconditional swapping erased the mu dependence
rather than merely degrading the statistics. Existing mu-ladder results are
invalid and need re-running; temperature ladders were unaffected, as was the
MPI ReplicaExchange, which selected _exchange_prob_mu all along.

A degenerate ladder now warns at the end of a run. A 100% swap acceptance sat
unremarked in the log of six consecutive production runs, so the all-accept
and never-accept tallies say so explicitly. Checked on the whole-run tally
rather than live, because a ladder whose replicas have not differentiated yet
legitimately accepts every early swap.

Also fixed:

- PermutationMove with n_swaps > 1 could apply k swaps and then return the
  "could not propose" sentinel when a later iteration drew a species absent
  from the system. The ensembles read that sentinel as "the atoms were not
  touched" and skipped the rollback, leaving the configuration out of step
  with E_old. Species counts are swap-invariant, so the usable species are
  now resolved once, up front. Both ensembles also restore their snapshot on
  the sentinel path rather than trusting the contract.

- Molecules whose centre of mass drifted above a CustomCell's top stopped
  being deletion candidates and dropped out of the per-species de Broglie
  count, inflating V/((N+1)Lambda^3) into the runaway insertion mode that
  docs/gcmc_acceptance_convention.rst describes. Cells grow a second
  predicate, is_point_exchangeable, which CustomCell overrides with the same
  dropped z upper bound that get_atoms_specie_inside_cell already applied to
  single atoms. find_molecules and the displacement one-way-door guard both
  use it, so candidacy and displacement regions always agree.

- AlchemiCalculator(energy_only=True) discarded 'forces' from the model
  config, which a pre-loaded MACEWrapper shares with every other calculator
  built from it, silently disabling FIRE relaxation in an AlchemiFCalculator
  depending on construction order. The combination now raises.

- GrandCanonicalEnsemble/CanonicalEnsemble.set_state restored the step count
  and exchange statistics from the incoming state. ReplicaExchange passes a
  full get_state() dict on every accepted swap, so the two ranks traded their
  swap tallies. Only the configuration travels now.

- GrandCanonicalEnsemble.write_outfile returned before reset_counters() when
  the outfile was disabled, silently turning interval_ratios() into
  total_ratios() for the rest of the run.

- MoveSelector abbreviated move names with a three-character slice, collapsing
  MoleculeInsertionMove, MoleculeDeletionMove and MoleculeDisplacementMove to
  one indistinguishable 'Mol'. Single-word moves keep their existing labels so
  old outfiles stay comparable.

- A missing entry in species_radii raised a bare KeyError from inside the
  free-volume sampler; it now names the species.

CI gains the flake8 job CLAUDE.md already documented.
An acceptance-equalized mu ladder for this system needs ~29 rungs (spacing
0.064 eV at the bare end down to 0.020 eV at high coverage, from
dmu = 1/sqrt(beta dN/dmu) on the measured isotherm). At ~460 atoms per replica
that is ~13k atoms in the relax batch, several times the whole-batch ceiling,
so the example could not run a correctly spaced ladder at all. chunk_size ties
peak memory to the largest chunk instead of the replica count.
A mis-spaced mu ladder fails silently: no error, no warning, and an output that
reads as plausible physics. Two things make it hard to catch, and both are now
written down.

First, the cumulative acceptance column cannot be used to judge a ladder. Every
replica starts from the same configuration, so early swaps are free and inflate
the tally permanently. A five-rung CO/CuPd run reported cumulative per-slot
acceptances of 28.6 / 19.6 / 5.9 / 2.0 / 0.0 %, which reads as a ladder that
merely mixes poorly at one end; in the run's second half three of its four pairs
accepted exactly zero swaps and four replicas were independent single-mu chains.
The halving test recovers the real rate from the log without extra
instrumentation.

Second, uniform spacing cannot work across a coverage range at all, because
dN/dmu grows with coverage while dmu stays fixed. The fix is
dmu = 1/sqrt(beta dN/dmu) from fluctuation-dissipation, with the rung count as
the integral of sqrt(beta dN/dmu). For CO on Cu375Pd30 at 400 K over
-1.8..-1.0 eV that is 29 rungs rather than 5, spacing 0.064 eV at the bare end
down to 0.020 eV at high coverage. Measured second-half acceptance of that
ladder: min 18 %, median 40 %, max 65 %, no dead pair, and a monotonic isotherm
from 0.1 to 57 CO.

The page carries a reference implementation, verified to reproduce that ladder,
for whoever lands the mu counterpart of utils.ladder.geometric_temperatures. It
also records the calibration caveat (the rule targets the mean-dN exponent, but
the realised <min(1, .)> acceptance runs higher, so the rung count is a safe
upper bound to trim against) and the two practical consequences of going wide:
chunk_size becomes mandatory, and the run gets faster in absolute terms because
a narrow ladder leaves the GPU idle.
Minor rather than patch: the release adds a public cell predicate
(`is_point_exchangeable`), adds a degenerate-ladder warning, and changes the
outfile acceptance-ratio header for molecule moves, which any downstream parser
of that column will notice.

Also restores the `[1.3.0]` compare link that was omitted when 1.3.0 was cut.
Add the CSI 2026 and AI4AM 2026 abstracts from the group's grand canonical
modelling work as evidence of use in the research impact statement, and
rebuild paper.pdf.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant