sergio-santos-group / ProtoSyn.jl

A Julia-based framework for molecular modelling
https://sergio-santos-group.github.io/ProtoSyn.jl/stable/
GNU Affero General Public License v3.0
58 stars 7 forks source link

2x speed up steepest_descent #51

Closed zznidar closed 1 year ago

zznidar commented 1 year ago

This PR improves performance of Steepest_Descent driver 2-fold.

There's no need to calculate already-calculated energy and forces. After recoverfrom!, both pose and backup_pose are two instances of an identical object. They have the same .e and .f. This means that we don't need to calculate energy and forces of backup_pose again in this iteration (https://github.com/sergio-santos-group/ProtoSyn.jl/blob/1abcc9fffb355e8937f5c28c01d7613a6d386e18/src/Core/Drivers/steepest_descent.jl#L185), because it was already calculated in the previous iteration on the pose object, copying it to the backup_pose (using recoverfrom!; either https://github.com/sergio-santos-group/ProtoSyn.jl/blob/1abcc9fffb355e8937f5c28c01d7613a6d386e18/src/Core/Drivers/steepest_descent.jl#L187 or https://github.com/sergio-santos-group/ProtoSyn.jl/blob/1abcc9fffb355e8937f5c28c01d7613a6d386e18/src/Core/Drivers/steepest_descent.jl#L190)

Since calculating energy and forces is the step that takes the longest time, this makes it ~2 times faster.

Could you please confirm that my understanding is correct?

JosePereiraUA commented 1 year ago

I think you are right. I suppose this is like this because in the past we used copy (see 985e0986b80ea40556637df7533d3426449cf7ec). This was before recoverfrom! existed and we haven't used steepest descent much since then.

Have you tested this in a simulation? If you think it's working fine, I won't have any problem in merging this PR! :D Thank you so much!

zznidar commented 1 year ago

Great! Yes, I have done a handful of simulations, they were the same before and after this change.