Add beta_jan25 energy function
Hi @roccomoretti and @lyskov,
@fdimaio and I have developed a new beta energy function called beta_jan25. This is an updated version of beta_nov16.
The aim of this PR is to add beta_jan25 to the rosetta source code.
We will shortly post a manuscript describing how we developed beta_jan25. They key updates are to the LJ potential. We identified steric clashing in proteins that were relaxed or designed using beta_nov16. We identified examples of this problem in a high-quality benchmark from the dualoptE protocol used to train the energy function. We then used this benchmark, and the others in dualoptE, to refit a small number of LJ parameters. The refitting largely eliminated the clashing problem, and beta_jan25 is as good or better than beta_nov16 when assessed on multiple benchmarks using validation data.
Please let me know if you have any comments or questions. I am also happy to share a draft of the manuscript. I am having Frank review my changes, and then I can ping one or both of you when Frank gives the thumbs up.
@fdimaio: could you please review my changes? And let me know if there are any additional changes I need to make? A few questions I had are:
- should genpot be "on top" of
beta_jan25? Or should we stick withbeta_nov16? See line 752 ofscore_function_corrections.cc - currently, the
-betaflag does not give the same results as either the-beta_jan25flag or the-beta_nov16flag. At least one reason for this is it seems to invoke thebeta_genpot.wtsfile. Is this the correct behavior?
Thanks!
Hugh
Looks good to me! @Haddox please let us know when this is ready to be merged. Thanks,
Thanks @lyskov!
@roccomoretti: Thanks for the suggestion to add tests to make sure beta_jan25 is invoked properly. After discussing it with Frank, we decided to add a test in tests/sfxn_fingerprint/scores/, which scores a set of input PDBs with a specific energy function and uses the output scores as a fingerprint. The test makes sure this fingerprint remains the same over time. I just added a version of this test for beta_jan25 in my last commit.
Frank also advised me that -gen_potential should stay the same (on top of beta_nov16), and we decided to have plain -beta invoke beta_jan25.
Updating -beta will cause some of the previous tests to fail because it now calls a new score function. Frank suggested I run the tests, and then Frank and I will look over the results to make sure any failing tests look reasonable (small changes in numbers due to the new score function). I will do that now.
fyi: I canceled regression tests comparison phase for 94d389a revision (looks like server could not determine base to compare to), @Haddox could you please merge-in latest upstream main and see if that fix the issue? Thanks,
Hi @roccomoretti and @lyskov,
@fdimaio and I wanted to check with you to see if this PR would be ready to merge.
To summarize the changes we've made:
- we added
beta_jan25to the source code - we added a test to
tests/sfxn_fingerprint/scores/to make sure thatbeta_jan25is called correctly - we updated
betato callbeta_jan25
Some of the standard tests are "failing" because their outputs have changed. The changes look reasonable to us, with differences in scores arising from updating beta.
Please let us know if you have any other questions or suggestions.
Thanks,
Hugh
Specifically, ga_ligand_dock, hbnet, and degreaser all use XML files with "beta.wts" ... this checkin changes beta.wts to the latest efunc.
@Haddox - LGTM! @roccomoretti do you have anything on your list before we can merge this? Thanks,
Looks like things need to be beautified. (python ../tools/python_cc_reader/beautify_changed_files_in_branch.py from the source/ directory)
Regarding the ga_ligand results change, does the beta_jan25 scorefunction pull in the ligand torsional changes from gen_potential, or are those omitted? Is running ga_ligand_dock with beta_jan25 appropriate? -- If you do want to run ga_ligand_dock with gen_potential rather than beta_jan25, then it might be appropriate to switch the integration tests to use best practices. (It looks like they are mixing the -gen_potential command-line flag with the beta.wts weights in the XML. I'm not sure if that environment/weight mismatch is an issue.)
@roccomoretti: I just ran beautify_changed_files_in_branch.py and pushed the results. It made a minor change to one of the files.
I believe that the beta_jan25 scorefunction does indeed pull in the ligand torsional changes from gen_potential. But @fdimaio can confirm. And I'll defer to his judgement about whether we should change beta.wts to beta_genpot.wts.
P.S. seems as though I pulled in a commit from main. Please ignore. Sorry if that was the wrong git flow.
@roccomoretti: regarding the ga_ligand tests, Frank advised me to update the weights files to use genpot weights. I just pushed those updates. Thanks for catching that.
Also, it appears that my branch now has conflicts that must be resolved. I think this might relate to the commit (1314848) that I accidentally pulled in. Could you please advise me on the best way to address this conflict?
Thanks!
You should be able to use the "Resolve conflicts" button on the Github website to change things. (Just select the contents of main)
Alternatively, just merge your branch with main (git fetch, git merge origin/main) and the resolve the conflicts locally (for files you shouldn't be changing with your branch, you can just do git checkout origin/main -- <filename>).
We use squash merges, so you don't need to worry about clean history (everything is squashed to a single commit anyway).
@roccomoretti: the updates to the ga_ligand tests have resolved the scoring differences. Now, the reported diffs for these tests show lines of the XMLs where I changed the weights file.
For some reason, the windows.cl.windows.build.debug test is now failing. The log indicates a permission error. It seems like this might be unrelated to the changes I'm making on this branch? Please let me know if you have any thoughts here.
And please let me know if you think this PR would otherwise be ready to merge. Since the last time I asked that question, I have ran the beautification script and updated the ga_ligand tests.
Thanks!
Hi @roccomoretti, just pinging you on this thread in case the notification got buried and in case a reminder would be useful. Thanks!