JuliaMolSim / Molly.jl

Molecular simulation in Julia
Other
371 stars 51 forks source link

fix bug in MC anisotropic barostat #138

Closed bondrewd closed 1 year ago

bondrewd commented 1 year ago

The length scaling factor in the anisotropic barostat is currently:

l_scale = SVector{D}(mask1 * (D == 2 ? sqrt(v_scale) : cbrt(v_scale)) + mask2)

However, since in the anisotropic case only one axis is scaled at a time, it should be replaced to:

l_scale = SVector{D}(mask1 * v_scale + mask2)
codecov[bot] commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: -36.33 :warning:

Comparison is base (0e80b6c) 73.04% compared to head (0cf3643) 36.71%.

:exclamation: Current head 0cf3643 differs from pull request most recent head 7402d2c. Consider uploading reports for the commit 7402d2c to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #138 +/- ## =========================================== - Coverage 73.04% 36.71% -36.33% =========================================== Files 34 34 Lines 4886 4834 -52 =========================================== - Hits 3569 1775 -1794 - Misses 1317 3059 +1742 ``` | [Impacted Files](https://app.codecov.io/gh/JuliaMolSim/Molly.jl/pull/138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMolSim) | Coverage Δ | | |---|---|---| | [src/coupling.jl](https://app.codecov.io/gh/JuliaMolSim/Molly.jl/pull/138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMolSim#diff-c3JjL2NvdXBsaW5nLmps) | `3.22% <0.00%> (-83.70%)` | :arrow_down: | ... and [26 files with indirect coverage changes](https://app.codecov.io/gh/JuliaMolSim/Molly.jl/pull/138/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMolSim)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

jgreener64 commented 1 year ago

Looks good, just rebase/merge recent changes I made to shorten the tests and I will merge this.