[WIP] biophysical_with_ic50 #113
Conversation
|
This pull request introduces 2 alerts when merging 51878e1 into c8cfda1 - view on LGTM.com new alerts:
|
| self.log_sigma_measurement = torch.nn.Parameter(torch.zeros(1)) | ||
|
|
||
| def g(self, func_value=None, test_ligand_concentration=1e-3): | ||
| return 1 / (1 + torch.exp(-func_value) / test_ligand_concentration) | ||
| def _get_measurement(self, delta_g, concentration=1e-3): |
There was a problem hiding this comment.
@yuanqing-wang : What units are you using here? It would be good to document them in the docstring!
There was a problem hiding this comment.
that's my code, I will add them
| delta_g : torch.Tensor, shape=(number_of_graphs, 1) | ||
| Binding free energy. | ||
|
|
||
| concentration : torch.Tensor, shape=(,) or (1, number_of_concentrations) |
|
|
||
| Returns | ||
| ------- | ||
| measurement : torch.Tensor, shape=(number_of_graphs, 1) |
There was a problem hiding this comment.
I think there's a typo in this equation.
If you want percent inhibition, you'll want something like
100 * (1 - 1.0 / (1.0 + concentration / torch.exp(delta_g))where concentration must be in Molar units.
This means that when delta_g << 0 (a tight binder), we have 100% inhibition, and when delta_g ~ 0, we should have 0% inhibition.
There was a problem hiding this comment.
So what I did was copy the function g from Josh's notebook ( https://gist.github.com/maxentile/fdc42d76c1125aff33f912fbc28edb1f )
Concentration is indeed assumed to be in Molar units in this code, and in our experiments.
Let me check out how your comments map to that code so I understand your comment better.
The original code in Josh's notebook is the following, with the math as explained beneat:
reference_concentration = 1 # molar
test_ligand_concentration = 1e-3
def g(f, c=test_ligand_concentration):
return 1 / (1 + np.exp(-f) / c)
@jchodera Can you explain the deviation from what you wrote above so I can parse my mistake?
There was a problem hiding this comment.
Also, while we're at it, I only ever got Josh's notebook as a reference for this function. Where can I look up the published version of this so I can check the math independently? I should have asked for this earlier to debug my own code, I rushed this too much.
|
So let me add some comments here. First, @yuanqing-wang it would be good to get into the habit of leaving descriptions about your PR, even when obvious. Second, I see you are trying to 'polish' my left behind version of the biophysical regressor, which I appreciate. But imho we should rethink a lot of our designs anyway for the models, as the other BiophysicalGP regressor 'runs' now but all of this has lost some modularity. I will look more into the PR to see the details. @jchodera : the units are my bad, @yuanqing-wang has just edited a piece of code I submitted in a rush without doc. We discussed the units in our own chat for pinot, they are in Molar for the parameter concentration to be passed. Since we have not gotten results with this that are satisfactory yet I would consider this still preliminary, I am also playing more with the other version of this code to see what we can get. give us a couple of days. |
| x_te=None, | ||
| delta_g_sample=None, | ||
| concentration_low=0.0, | ||
| concentration_high=1.0, |
There was a problem hiding this comment.
What do these parameters mean? Can you document them in a docstring?
We definitely do not want a uniform distribution over IC50 as a prior. It should be uniform over log10(IC50), over a meaningful physical range. You can see what a sensible range is from this paper:
https://journals.plos.org/plosone/article?id=10.1371/journal.pone.0061007
So maybe 0-14 in -log10(IC50).
There was a problem hiding this comment.
Thanks John, I think this was sth @yuanqing-wang just wanted to ask me stuff about that he hacked in just today.
@yuanqing-wang can you elaborate on your thinking? Ideally also with PR comments when you add a new function like this?
There was a problem hiding this comment.
Sorry @jchodera this is just a very immature draft I had in order to discuss some ideas. For the IC50 I was only thinking about how can we integrate that into the graphical model without using the Cheng-Prussoff equation. So I'm thinking about doing the titration curve here from concentration_low to concentration_high. But I realized that I can only do sampling this way but not get the distribution.
|
This pull request introduces 3 alerts when merging fffca5b into c8cfda1 - view on LGTM.com new alerts:
|
No description provided.