Revive RDF liquid target - #323
Open
adw62 wants to merge 3 commits into
Open
Conversation
Ports the RDF (radial distribution function) matching feature from leeping#109 onto current master, and fixes several issues found along the way rather than a literal replay of the old diff: - Fix a snapshot-ordering bug in OpenMM.molecular_dynamics(): RDF was being computed from the previous iteration's state/box_vectors (read before they were refreshed for the current iteration). - Make property_results['RDF'] follow the same (calc, std, grad)-dict-per-phase-point shape as every other liquid property, keyed per RDF name, instead of a bare list of RDF objects -- the pure_num_grad path (added to master since 2018) generically assumes that shape and would otherwise break when combined with an RDF target. - Make the npt_result.p unpacking in Liquid.get_normal() tolerant of older 17-field pickles (written before this feature existed), so existing cached results and the checked-in test_liquid fixtures keep working. - Fix an RDF/compute_msd naming collision (self.MSD list attribute shadowing the MSD() method) by renaming the method to compute_msd(). - Fix an int()-truncation bug in RDF bin-count computation that could drop a bin due to floating point error; use round() instead. - Add an explicit error if an RDF target is combined with a repeated evaluation at the same parameter values (adapt_errors), since RDF snapshot data isn't threaded through self.AllResults across iterations the way other properties are -- this was silently wrong in the original PR and is left as a follow-up rather than fixed here. - Add mdtraj (a direct import for the new RDF class) to the conda test environments; it was previously only pulled in transitively. - Add unit tests for the RDF class (unit conversion, MSD computation, and an end-to-end Pairs()/Calc() check against a synthetic 2-water system) since the original PR's studies/RDF_test example was accidentally deleted by cleanup commits on the PR branch. All existing src/tests/test_liquid.py, test_openmmio.py, and test_parser.py tests still pass, including the pre-existing 17-field-pickle fixture. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012p4S8e1nRubXRqc6HQU2Tn
studies/030_rdf_lj_fluid_validation fits a single Lennard-Jones sigma against a synthetic target RDF (generated by directly simulating the ground-truth fluid, not fabricated) and checks that the optimizer recovers the true sigma -- in opposite directions for two different targets from the same wrong starting guess. Both converge to within 1% of the true parameter in a couple of minutes on GPU, exercising the full RDF pipeline (rdf.dat parsing, MDTraj sampling during NPT MD, MBAR reweighting, analytic gradient) rather than just its unit- tested pieces. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012p4S8e1nRubXRqc6HQU2Tn
Open
- npt.py: don't crash on non-OpenMM engines (RDF_data key was only returned by openmmio.py); error clearly if rdf.dat is present on an unsupported engine instead. - openmmio.py: use each box vector's own length for RDF trajectory construction instead of assuming a cubic box (breaks anisotropic_box); skip RDF parsing/pair-building for the gas-phase (non-pbc) engine instance to avoid wasted work on a stale pairs.pdb. - liquid.py: require mdtraj at RDF target setup instead of failing later with a NameError; error clearly when a cached npt_result.p predates RDF support but an RDF target is now configured; validate rdf.dat covers every phase point before computing MSD; degrade RDF targets gracefully (skip that evaluation's RDF term with a warning) instead of hard-crashing when combined with adapt_errors/repeats.
Author
|
Hey @leeping this should now be up to date, works in my testing with LJ fluids. Limitations are it does not work with the adapt_errors feature and it only works with OpenMM. |
Author
|
@AnthoniAlcaraz could try this? if you are using OpenmMM hopefully can work for your use case :) |
Hi @adw62 , Thanks a lot for updating this feature! I have an OpenMM setup running on GPU, so I'm well positioned to test this. I plan to use this PR to parameterize a classical force field for molecular oxygen, targeting experimental RDFs and other thermodynamic properties (density, enthalpy, etc.). I will pull this branch, run the LJ validation example first to confirm my environment setup, and then run tests on my system. I'll report back here with feedback at least for the LJ validation! |
Owner
|
Thanks so much - please let me know when it’s ready for me to run tests
locally and merge. We can plan for it to be included in the 1.11 release
(of course you may run it from source in the meantime).
…On Tue, Aug 25, 2026 at 7:25 PM AnthoniAlcaraz ***@***.***> wrote:
*AnthoniAlcaraz* left a comment (leeping/forcebalance#323)
<#323 (comment)>
@AnthoniAlcaraz <https://github.com/AnthoniAlcaraz> could try this? if
you are using OpenmMM hopefully can work for your use case :)
Hi @adw62 <https://github.com/adw62> ,
Thanks a lot for updating this feature!
I have an OpenMM setup running on GPU, so I'm well positioned to test
this. I plan to use this PR to parameterize a classical force field for
molecular oxygen, targeting experimental RDFs and other thermodynamic
properties (density, enthalpy, etc.).
I will pull this branch, run the LJ validation example first to confirm my
environment setup, and then run tests on my system. I'll report back here
with feedback at least for the LJ validation!
—
Reply to this email directly, view it on GitHub
<#323?email_source=notifications&email_token=AAK76GFMRMYDW4NKK5JSGGT5LYU7FA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBRHA4DKMRRHE22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5418852195>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAK76GHOECVJZCFC2LXHYY35LYU7FAVCNFSNUABEKJSXA33TNF2G64TZHMYTCMZQGI3DEMZ3JFZXG5LFHM2TENBQGEZDMOBRG2QXMAQ>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AAK76GDVJE36B3FTNXB46TL5LYU7FA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBRHA4DKMRRHE22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/AAK76GHNODDYO6NUQQUI6JD5LYU7FA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBRHA4DKMRRHE22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I have tested this PR using OpenMM on a GPU for the The optimization runs smoothly and reliably converges in under a minute (~50-57 seconds). Thank you very much! |
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
3 participants
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.
Ports and fixes the RDF liquid target from #109 (open since 2018), plus a closed-loop GPU validation example (studies/030_rdf_lj_fluid_validation) that fits a Lennard-Jones sigma against a synthetic RDF and confirms the optimizer recovers the true parameter to <1%. Closes #109.