-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cannot reproduce wB97M-D3(BJ) energies and forces #50
Comments
Just a follow up: I tried the same calculation using version 1.4.1 and the energy was |
Hi @pablo-unzueta , thank you for raising the issue, we did use The main reason for mismatch in energies is not converting the coordinates from Also, for water the energies (in hartree) I see in the hdf5 file are
I couldn't find Edit: sentence |
Hi @pavankum, I started working on this again today. I think the issue comes from the downloader.py script. When I use it, my energies for water are the following, before I start writing them as ase files:
However, if I download the SPICE.hdf5 file directly from the releases page, I reproduce what you posted: Attached is the downloader.py script I used. Please let me know if I goofed something :)
Edit: Code formatting |
I think you're running into #44. Make sure you have the latest version of the downloader script. We're going to be suffering from that error for a long time. :( In retrospect, it was a big mistake to put the lower level of theory into the same dataset on QCArchive. Is there a way we can move it out or delete it? Working around it in the downloader is clearly insufficient, because there are still people with the old version of the script on their computers. |
Thanks everyone! I'll close this issue for now. |
@peastman the next release of qcfractal (expected late December) may allow data deletion within a set, right now I think the dataset collections are immutable, or need Ben's help in modifying the database at root level. |
We can't afford to wait until late December. People are downloading it right now. Can we escalate this with the QCArchive team? |
@peastman putting the lower level of theory into the same dataset wasn't the wrong move here; though it is possible to create a new dataset for each additional level of theory / compute spec you want to run, it quickly becomes even more of a zoo to navigate. Can we quantify how bad a problem this is first? The new release of this repo with the downloader update has only been out two weeks, so it's not clear to me that performing surgery on the database it's targeting is warranted. Also: we'll likely need another update to the downloader script once the Perhaps instead of a downloader script, an archival service such as Zenodo or OSF is a better approach for versioning and dataset stability? That would also mint DOIs and satisfy journal requirements. QCArchive, despite its name, is currently more of a compute engine with attached storage, and versioned exports are more appropriate for archival. |
About as bad as it gets. It means that at this moment there may be people running the downloader and getting corrupt versions of the dataset. And there's no way for them to tell it's corrupt, and those corrupt dataset files may sit around on their computers for years and get used to train models that they publish years from now. So very, very bad.
It's too late for any change like that to fix the problem. The downloader script is out there. People have already cloned the original version of the repository. They've read the preprint, which instructs them to use the downloader. So any other distribution mechanism we create in the future won't prevent people from running the downloader that's already out there and getting wrong results. The only possible fix is to make sure that the original version of the downloader produces correct results (as it did when it was originally written, before the alternate data was added to QCArchive). |
Can you help me understand the degree of concern, @peastman? There was a bug in the unpublished SPICE 1.0 and 1.1 downloader scripts (but not the downloadable artifact that we generated) that results in some download attempts getting a different level of theory. The 1.1.1 version in the manuscript has fixed the nondeterminism bug a week after the manuscript was posted to arXiv. The repo has only been forked twice, by @peastman and @pavankum, so we don't even need to worry about forks that out of sync. In order to be impacted, someone would have to have
It's hard for me to understand how there can be that large a number of people impacted by this issue, or how it could result in erroneous publications given how unlikely it is for someone to make it all the way to publication with the wrong version of the dataset. We can certainly tweet about the fix to reach the same audience that saw the original tweet if this is a concern, which should further minimize the issue. And if QCPortal ends up being able to make the default choice deterministic, that problem will end up fully closed. People are using OpenMM with known bugs in it right now. But we don't try to erase historical versions of OpenMM just because there were bugs in it. We do our best to disseminate the bugfix so that our user community knows about it. |
Correct. But that's not an unusual thing, given that a lot of data fields are only available through the downloader.
The time window is much larger than that. The 1.0 release (which included the downloader script) was created on July 12. The first data at the lower level of theory was added on Aug. 21. The 1.1.1 workaround was not added until Sept. 28. That's 2.5 months during which someone could have cloned the repository and gotten a version without the workaround. You're assuming the preprint is the only way someone could have learned about it. But given how much we've all been talking about this project and how many people we've pointed to the repository, that isn't a safe assumption. So this affects anyone who cloned the repository on or before Sept. 28, and ran (or will run in the future) the downloader script any time after Aug. 21.
Correct. But then, why would they? When I download a piece of software or dataset I want to use, I don't usually keep checking their issue tracker afterward unless I specifically discover a problem.
Correct. Again, why would they? Unless they were checking back, they would have no way of knowing there was a newer version.
We definitely know of at least one person who encountered the problem, because he opened this issue. How many others are there? We have no idea. But since one person encountered it, it's likely other people have (or will) too.
That would be an acceptable solution. But again, this is urgent. People may be downloading corrupt datasets at this moment. We can't wait months for a fix. |
We did very little to publicly advertise this repository until the preprint posted because there was insufficient information to understand what it contained until the manuscript was available. And the repo has only been cloned by 11 people in the last two weeks after considerable public advertising.
To find the citation associated with the dataset. Citation information was only added on Sep 26, just two days before the 1.1.1 fix was released. To publish with the incorrect results, someone would had to discover the repo, clone it prior to Sep 28, and access the README in the 48 hours that the citation was posted but the updated release was not yet made. Given the cloning rate of 0.8 clones/day even after the manuscript is posted, this seems extremely unlikely. What if we publicly announce the 1.1.1 bugfix on twitter (OpenMM, and have @giadefa and myself retweet) and the README? That seems like it will reach nearly all the likely audience here. |
Hello OpenMM team,
I'm trying to reproduce some of the energies in the dataset.
Input file used:
Energy from Calculation:
-76.1624479876510065
Energy from SPICE.hdf5:
-76.42840576171875
One caveat is that I am using Psi4-1.6.1 and not 1.4.1. But I wouldn't expect a 0.3ish hartree difference.
Pulled from SPICE.hdf5 file used after using downloader.py and writing an ase style xyz file:
Found a previous issue using wB97M-D3(BJ) using version Psi4-1.4.1. I don't know if this affects it: psi4/psi4#2279
I'm currently installing Psi4 version 1.4.1 to rerun my test.
Can you point me in any other directions? Thanks
The text was updated successfully, but these errors were encountered: