Features/rmsd rmsf #381
Features/rmsd rmsf #381
Conversation
| def calculate_rmsd(simulation, reference_simulation) -> float: | ||
| positions = simulation.context.getState(getPositions=True).getPositions(asNumpy=True) | ||
| reference_simulation_positions = reference_simulation.context.getState(getPositions=True).getPositions(asNumpy=True) | ||
| diff = positions - reference_simulation_positions | ||
| rmsd = np.sqrt(np.mean(diff**2)) | ||
| return rmsd |
There was a problem hiding this comment.
I dont like that these take simulation objects rather than the positions themselves. We should be able to decouple the functionality of obtaining the information need for rmsd and calculating rmsd. This function does more than just calculate. Please refactor
There was a problem hiding this comment.
to further motivate this, I would like to iterate over many frames of the simulation, rather than a single one. I want this function to not need a simulation object at all unless needed
There was a problem hiding this comment.
also, there are different ways (likely more accepted ways) to do this calculation. see:
- https://docs.mdanalysis.org/1.1.1/documentation_pages/analysis/rms.html
- https://userguide.mdanalysis.org/stable/examples/analysis/alignment_and_rms/rmsf.html
these are related to alignment. I have also done this in the past and you can't just simply take the rmsd, but you need to check if the number of atoms is the same because it actually does change if you don't do the same processing to the molecule
| return False | ||
|
|
||
| # Check to see if we have a logging resolution of 10 or better, if not the run is not valid | ||
| if (self.log_file['#"Step"'][1] - self.log_file['#"Step"'][0]) > 10: | ||
| return False | ||
|
|
||
| return True | ||
| return True, rmsd |
There was a problem hiding this comment.
I don't like this - why does the method need to return rmsd here instead of doing the calculation somewhere else?
No description provided.