Closed JaydevSR closed 1 year ago
Base: 83.87% // Head: 83.74% // Decreases project coverage by -0.13%
:warning:
Coverage data is based on head (
93d16bb
) compared to base (47f3e1f
). Patch coverage: 84.78% of modified lines in pull request are covered.:exclamation: Current head 93d16bb differs from pull request most recent head ab4b6e6. Consider uploading reports for the commit ab4b6e6 to get more accurate results
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Looks like a great start. If the lattice formation is clearer in 2D you could do that. I think the copy()
is okay for the moment, profiling would help point to the slow steps though.
Use of neighbor finder for calculating potential energy at each step?
I think call find_neighbors
every step and use the neighbours, as the other simulators do. In many cases, for example with large trial moves, not using a neighbour list would be more efficient since neighbour calculation would need to be run every step. It can be left to the user though since the default is NoNeighborFinder
.
sample the torsional angle of ethane
I can look at this myself after the PR is merged if it's easier.
A couple of other points:
potential_energy
only needs to be calculated once per step, with E_old
being updated at the end of each step.ReplicaExchangeLogger
that logs energies and acceptance/failure.randn
, dividing by the norm to get a unit vector, and multiplying by the distance drawn from the appropriate distribution.It would also be nice to have a logger similar to the ReplicaExchangeLogger that logs energies and acceptance/failure.
So, for the logger I am thinking that instead of logging the energies for which we already have a logger, we can log the value of $\displaystyle\frac{E}{k_B T}$ i.e. the argument of the Boltzmann rates which makes sense for a Monte-Carlo simulation. What do you think? Also, should I just log the number of acceptances and selections or also a bit array of when the acceptances occur?
A 2D simulation which shows lattice formation with 100 atoms in 4nm square boundary:
Similarly for 3D with 4nm boundary and 100 atoms:
Yes I think logging that value sounds good. I would log the bit array of when the acceptances occur too.
Now the question remains is where the logger would go. If we put it in loggers named tuple in the System type, then we cannot pass in info about whether the acceptance occurred or not. And also, it would recalculate any potential energy that we have already calculated in the metropolis part. In the replica exchange, I added a field to replica system. So, should there be a similar optional thing for the System type, like a montecarlo_logger
field?
This seems to be a general problem when writing simulator-specific loggers. I think we should change the arguments of the run_loggers!
and log_property!
function definitions to accept additional kwargs...
. In this case we could then pass the acceptance info and potential energy as keyword arguments to run_loggers!
and use them in the relevant log_property!
function. I think this should mostly be a case of adding kwargs...
to the keyword arguments of functions in src/loggers.jl
.
I have added all of the things that are needed for the PR ✌️. Please tell me if some more tweaks are needed or if I missed something.
I have added tests for acceptance rate (test for bad cases), avg potential energy (should decrease from a random config at start to end for a repulsive system), and the mean nearest neighbor distance (for a repulsive system it should not be less than the Wigner-Seitz radius and for the given boundary it is not possible for it to be 2 times the Wigner-Seitz radius).
Great, thanks for this.
This PR adds some simple Monte-Carlo functionality i.e., Metropolis Monte-Carlo for molecules. I was initially thinking of writing the simulator in
simulators.jl
but there may be some monte-carlo specific code (like the random translation trial moves here) that doesn't fit there so I added a new file.Some things that need to be done before merge:
copy()
which is not very efficient, any suggestions for improvement?I have run a test simulation given below:
This is what the simulation looks like:
https://user-images.githubusercontent.com/59474411/199914858-3d8d077e-9701-4ad5-abcd-48c06018acc4.mp4
As expected the atoms are arranging into a lattice (sort of). Another simulation that was suggested by @jgreener64 was to sample the torsional angle of ethane. I am not quite sure how to set that up so some help will be appreciated.