samplchallenges / SAMPL8

Challenge details, inputs, and (eventually) results for the SAMPL8 series of challenges
https://samplchallenges.github.io
MIT License
22 stars 11 forks source link

pKa predictions from ChemAxon #40

Closed dhimanray closed 3 years ago

dhimanray commented 3 years ago

I added the pKa predictions from ChemAxon

davidlmobley commented 3 years ago

@dhimanray This adds several other files at the same time, e.g. an analysis script, as well as some sample user input and the experimental values (?). Did you intend to add those in the same PR?

Usually it is best to make each PR make a single logical group of changes that can be described in the title, so in this case I was expecting this one to only bring in the ChemAxon predictions (and clearly note that these are reference calculations) but not make changes to the analysis or experimental values.

Also, it would be good to update the relevant README.md file at the same time to explain what's changed here. Particularly, you'd probably add a README.md file in https://github.com/samplchallenges/SAMPL8/tree/master/physical_properties/pKa/relative_microstate_free_energy_predictions which explains that all the files come from submissions except the ChemAxon one, which is a reference calculation done aftterwards. Then you should also update the changelog in the base level README.md (https://github.com/samplchallenges/SAMPL8/blob/master/README.md in the part "Changes not in a release") to mention that you added this, and on what date it was added.

dhimanray commented 3 years ago

@davidlmobley I removed the analysis files from the PR keeping only the pKa chemaxon data and the updates in the README files.