Skip to content

[WIP] engine_masses feature; use in Gromacs engine#1119

Open
dwhswenson wants to merge 1 commit intoopenpathsampling:masterfrom
dwhswenson:external_masses
Open

[WIP] engine_masses feature; use in Gromacs engine#1119
dwhswenson wants to merge 1 commit intoopenpathsampling:masterfrom
dwhswenson:external_masses

Conversation

@dwhswenson
Copy link
Member

Apparently Gromacs snapshots didn't have masses associated? See #1117. I was pretty sure we've used masses here before, but can't find an example of it.

Note that the better long-term solution for the needs of #1117 will be an engine.randomize_velocities -- we already have the machinery to use this, but haven't actually implemented any examples yet.

@codecov
Copy link

codecov bot commented Aug 9, 2022

Codecov Report

Merging #1119 (61d0c3e) into master (965d8a2) will decrease coverage by 0.01%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master    #1119      +/-   ##
==========================================
- Coverage   81.75%   81.74%   -0.02%     
==========================================
  Files         142      143       +1     
  Lines       15612    15622      +10     
==========================================
+ Hits        12764    12770       +6     
- Misses       2848     2852       +4     
Impacted Files Coverage Δ
...athsampling/engines/external_snapshots/snapshot.py 100.00% <ø> (ø)
openpathsampling/engines/gromacs/engine.py 94.08% <50.00%> (-1.63%) ⬇️
openpathsampling/engines/features/engine_masses.py 66.66% <66.66%> (ø)
openpathsampling/engines/features/__init__.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@sroet
Copy link
Member

sroet commented Aug 9, 2022

Leaving a comment here to keep the issue clean, but about

We'll need to be able to correctly randomize velocities, which requires both creating random velocities and then applying constraints. I don't think we have a way to apply constraints to a Gromacs snapshot yet. These are things that would be good to add, but I probably can't get to this in the very near future.

I am pretty sure that was fixed in PyRETIS by letting gromacs run a 0 step simulation, wouldn't a similar solution work for OPS?

@dwhswenson
Copy link
Member Author

@sroet : Yes, I think that is the approach to take, but implementing and testing it will take a bit of time, and is beyond what I can promise before the end of this month. (Goal for the next two weeks is to get 1.6 released before my ACS talk on Aug 24; I'm just glad to finally be back to working on OPS!)

@sroet
Copy link
Member

sroet commented Aug 9, 2022

(Goal for the next two weeks is to get 1.6 released before my ACS talk on Aug 24; I'm just glad to finally be back to working on OPS!)

Good luck with your talk! Just ping me if you want my eyes on something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants