Repository navigation
Review follow-ups: runtime reference mu, free-volume floor, article example settings - #31
Merged
Merged
Conversation
Replica exchange: - Add the de Broglie cross-term 3*sum_s(dN_s)*ln(Lambda_i/Lambda_j) to the grand-canonical swap criterion in both BatchedReplicaExchange and the MPI ReplicaExchange (lambda_dbs now rides on get_state). Mu ladders were already exact (the term cancels at shared T); temperature and joint ladders with fluctuating N were biased. - Reject non-finite swap exponents with a warning instead of silently inverting the NaN behavior (the old min(1, exp(nan)) accepted every swap). - Warn per rung on zero second-half swap acceptance (midpoint tally snapshot): the whole-run check could not see a ladder dead at one rung. - Report swap attempts, not doubled per-slot tallies, in the degeneracy warning, and emit it as a single log record. - Refuse factories that share atoms, move_selector or cells across replicas under batching. State and cells: - set_state restores step and exchange tallies when present (lossless restart round-trip); ReplicaExchange strips them on swaps instead. - is_point_exchangeable defaults on BaseCell, and the molecule moves fall back to is_point_inside for duck-typed cells (pre-1.4.0 contract). Alchemi calculators: - Raise when head= is passed with a pre-loaded MACEWrapper (it was silently ignored, evaluating multihead models on the pretrain head). - Drop the impossible workaround from the energy_only guard message. - Refuse forces-stripped models in AlchemiFCalculator and in run_md. Docs: - Ladder-spacing reference: include the mu_max endpoint (fencepost), sort descending isotherms, clip noise-negative dN/dmu.
- eval() the wrapper on local checkpoint loads: the nn.Module train-mode default made MACE run the force autograd with create_graph=True and retain_graph=True on every FIRE and Langevin step (values unaffected) - drop the stale manual requires_grad in AlchemiCalculator and run energy_only forwards under no_grad, restoring the graph-free forward (peak 242 vs 342 MB per forward on Ag225) - zero FixAtoms forces after the bootstrap compute so step 0 sees the same frozen-force state as every hook-managed step - remove the donated_buffer=False workaround: the crash it papered over was a train-mode symptom; verified with compile+cuEq on the alias and local checkpoint paths across varying-N GCMC steps - add NaNDetectorHook to the FIRE and Langevin loops and warn when a relaxation returns at the step cap (unconverged-energy bias trap) - correct the FIRE dt docstring (nvalchemi defaults dt_max to 10*dt, unlike ASE's 1 fs cap) and document why AlchemiBrownianMove needs no position wrapping Verified: verify_compact_parity PASS (max compact-vs-serial delta 0.0002 eV, FixAtoms rows bit-identical), 198 tests pass, flake8 clean. Post-fix numbers on the RTX 5090: 3.7 s/GCMC step at 3.7k atoms (4.9 GiB allocated), single-relax ceiling 32k atoms.
…e estimates - Add mcpy.utils.reference_mu: derive_mu_bulk (lattice-constant scan with parabolic fit) and derive_mu_gas (E(O2)/2). Examples now default mu_Ag and mu_O to values derived from the run's own checkpoint instead of the hardcoded -2.99/-4.91, which silently changed meaning across potentials. - Clamp the MC-sampled free volume in CustomCell, SphericalCell, and DomeCell to one sample point's worth of the region, so a fully covered region cannot report 0 A^3 and turn the deletion prefactor into inf/nan. - Align example defaults with the article simulations: relaxing AlchemiFCalculator, mace-small-density checkpoint, 300-800 K ladder and delta_mu_O=-0.3 in re_gcmc_batched; Octahedron(9,3) in re_gcmc_nano_alchemi; fmax=0.05 with 100 relax steps in re_gcmc_co_cupd_batched. - Add tests for the reference-mu helpers and the zero-free-volume floor.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mcpy.utils.reference_mu:derive_mu_bulk(lattice-constant scan with parabolic fit) andderive_mu_gas(E(O2)/2). Examples now derivemu_Ag/mu_Ofrom the run's own checkpoint instead of hardcoded values that silently changed meaning across potentials (c2bc7d9).CustomCell/SphericalCell/DomeCellat one sample point's worth of the region, so a fully covered region cannot report 0 A^3 and turn the deletion prefactor into inf/nan.AlchemiFCalculator+ mace-small-density checkpoint, 300-800 K ladder,delta_mu_O=-0.3inre_gcmc_batched;Octahedron(9,3)inre_gcmc_nano_alchemi;fmax=0.05, 100 relax steps inre_gcmc_co_cupd_batched.Test plan
pytest tests/— 205 passed (includes new tests for reference-mu derivation and the zero-free-volume floor)flake8 mcpy/cleanexamples/re_gcmc_batched.py(2 replicas, 20 steps, GPU): derived mu(Ag) = -2.8172 eV, mu(O) = -4.9093 eV (matches known E(O2)/2 for the checkpoint); insertions proceed atdelta_mu_O=-0.3; free-volume floor fired once and the run continued; outfiles and trajectories written