Replace global_vars with frozen SimulationParameters dataclass#17
Merged
Conversation
Eliminate module-level mutable state (temperature, rate_prefactor, kT) that prevented running simulations at different temperatures in the same process. Introduce a frozen SimulationParameters dataclass holding temperature and rate_prefactor, with a derived kT property. This is threaded explicitly through the call chain: Simulation -> Lattice -> Jump and Transitions. - Add lattice_mc/constants.py with k_boltzmann physical constant - Add SimulationParameters frozen dataclass to simulation.py - Simulation.__init__ now requires a SimulationParameters argument - Jump and Transitions accept params as keyword-only argument - metropolis() takes kT as an explicit parameter - LookupTable reads kT from lattice.params - Delete lattice_mc/global_vars.py - Update all tests, examples, and docs
- Add RuntimeError guard in Lattice.potential_jumps() when params is None, with a clear error message - Fix constants.py module docstring placement (was after import) - Add trailing newline to notebook - Add test for the params guard
Validate that temperature and rate_prefactor are positive in __post_init__. Fix pre-existing TranstitionsTestCase → TransitionsTestCase typo. Addresses Copilot review comments on PR #17.
Simulation.setup_lookup_table() advertised 'coordination_number' as a valid hamiltonian but LookupTable only accepts 'nearest-neighbour', so passing 'coordination_number' always raised ValueError at the LookupTable level. The lookup table cannot support coordination-number energies because the energy change is a many-body calculation that depends on the full local microstate, not just (l1, l2, c1, c2). Coordination-number energies remain fully supported via on-the-fly computation in Jump.coordination_number_delta_E() — no lookup table is needed since cn_energies are themselves dict lookups. Add integration test for simulation with cn_energies.
The bare ValueError made debugging harder when an unsupported hamiltonian was passed. Now includes the invalid value and the list of accepted values in the error message.
- SimulationTestCase docstring: "Species" -> "Simulation" - test_jump_raises_BlockedLatticeErrror: extra 'r' removed - LookupTable docstring: "nearest-neigbour" -> "nearest-neighbour"
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
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
temperature,rate_prefactor,kT) fromglobal_vars.pythat prevented running simulations at different temperatures in the same processSimulationParametersdataclass holdingtemperatureandrate_prefactor(with derivedkTproperty), threaded explicitly through the call chainSimulation.__init__now requires aSimulationParametersargument (no defaults)lattice_mc/constants.pywith thek_boltzmannphysical constantlattice_mc/global_vars.py