samplchallenges / SAMPL6

Challenge inputs, details, and results for the SAMPL6 series of challenges
https://samplchallenges.github.io
MIT License
52 stars 32 forks source link

Add empirical reference calculations to logP analyis. #88

Closed MehtapIsik closed 5 years ago

MehtapIsik commented 5 years ago

These PR adds a variety of empirical logP prediction methods (submitted by Thomas Fox) to logP challenge analysis.

bergazin commented 5 years ago

Looks good to me.

davidlmobley commented 5 years ago

@MehtapIsik mostly looks good, except I think you've also updated lots of plots that haven't changed, at the same time? Specifically (for example) the absolute error plots for all of the individual submissions.

I'm guessing you just re-ran the analysis script, but then committed all of the files whether they had changed or not?

MehtapIsik commented 5 years ago

Yes, correct. My script replotted all those. Even if nothing changes, something changes in PDF binary files. So I committed all those new plots.

davidlmobley commented 5 years ago

Right, so the way I would have done it would have been to only commit files which changed so that the revision history doesn't look like we updated everything.

If it's too much trouble to roll that back we can go ahead with what you've committed, but I worry it will make the revision history confusing to have all these plots "updated" after the challenge.

MehtapIsik commented 5 years ago

I know it is crowding the revision history but it is too hard to differentiate between PDFs that were really updated and PDFs that look like they were updated. I am afraid to cause a mixture of new and old files and introduce mistakes if I try to avoid these. It is better to merge as is.