search

rinikerlab / reeds

This pipeline is executing a RE-EDS run for relative free energy calculations. It can be used for example for calculation of hydration free energies or ligand binding free energies.
https://rinikerlab.github.io/reeds/
MIT License
30 stars 8 forks source link

Change - Preparation of s-optimization input (ported from gitlab) #16

Closed SalomeRonja closed 2 years ago

SalomeRonja commented 3 years ago

by @candidechamp :

The code converting the output of the energy offset analysis seems to produce .imd files which don't have the proper number of s-values for s-optimization step.

Right now, the code ensures that if you had 11 s-values at the end of the energy offset estimation (uniformly distributed on a log scale), then you should also have 11 s-values for the first step of the s-optimization.

However, now that we are using the SSM approach, it will enforce that we have N s = 1 replicas (where N is the number of end states). The code therefore redistributes the 11 - N other s-values uniformely, which means that the s-distribution of lower s-values is smaller. (In my specific example with CHK1, N = 5, so we are left with only 6 replicas with s != 1).

I think it would be better to add the replicas with s = 1 such that we keep all of the other s-values just as they are. We would therefore keep the same s-distribution, and add (N-1) replicas for s = 1.

What do you think @SchroederB @SalomeRonja @epecora ? This is what seems intiutive to me (and that's why I file it as a bug), but maybe you think differently? Reducing the number of s-values also in principle means we need to do even more s-optimization.

I will fix this I you give me the thumbs up that it is the functionality that we want to have! (I had planned to change this part of the code anyways, as this s-distribution would have to change if we find out that using a different initial s-distribution could lead to a shorter s-optimization phase).

SalomeRonja commented 3 years ago

comment by @SchroederB :

Hi @candidechamp , hm I see your point. I think one can do that, but I would not call it Error ;) rather Improvment or Change Suggestion, as it is not an error. I intended to try if one can end up with less replicas in total, if you more optimize on a smaller s-distribution distribution.

For your approach trying to optimize from step c_already, I see why changing this makes sense.

SalomeRonja commented 3 years ago

comment by @candidechamp :

@bschroed Agreed! I will take care of this in the next few weeks.

RiesBen commented 3 years ago

@candidechamp This was resolved by you right? Can we close the issue?

candidechamp commented 3 years ago

@SchroederB I think my change for this was on a branch that I didn't merge yet. But I'm not sure anymore. I'll check and write an update here.

candidechamp commented 3 years ago

Just a comment for clarity if anyone comes back to this in the future.

This issue will be addressed on the development branch candide (I should be implementing this today).

RiesBen commented 3 years ago

@candidechamp If you solved the issue on your branch, could you please add a Done label to it? This makes it more clear, if work is needed here or not.

candidechamp commented 2 years ago

The changes are finally implemented back into the main branch with #65