-
Notifications
You must be signed in to change notification settings - Fork 4
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
store_wavefunction not propagated #244
Comments
Hi @lilyminium good spot, I see that we try to propagate it to qcfractal here when making the |
QCElemental right now doesn't support protocols for the gradient calculations. The structure in QCElemental is a bit different than QCFractal: QCFractal reuses a full QCSpecification object, but QCElemental has its own, which unfortunately doesn't have protocols: https://github.com/MolSSI/QCElemental/blob/e942b810f1681b7d38209d0fed55b49954e6e4b5/qcelemental/models/procedures.py#L56 So I fix the protocols to be the default to at least maintain consistency in the DB (I should probably add a comment there). This was a huge pain during the migration from the legacy version to the current version. This is part of a QCElemental refactor which has been a long time coming, unfortunately. |
Since I'm taking over maintenance of QCSubmit, I took some time to wrap my head around this. Here are notes, maybe just for myself, though I'd really appreciate correction if I'm misunderstanding something:
So I think the right solution is that the |
Largely correct, with a few more clarifications:
I vaguely plan (with no timetable) to have this built in, since it will be very common: MolSSI/QCFractal#748 Another option is to overhaul the qcelemental models to take in two specifications, but that would be a bit more disruptive.
I wouldn't be so optimistic :) That value pertains only to the driver (which is typically energy, gradient, hessian). There's an awkwardness when we allow that to be set by the user for an optimization, because it is largely ignored - whether an energy, gradient, or hessian is run is controlled by the optimization software, not the user. So what would it mean to submit an optimization with
It's been on the table for a good while now, along with some qcelemental/qcschema splitting ideas. They just end up taking a back seat to other pressing issues.
I agree. The current behavior is technically correct but hidden. So raising an exception is better than silently changing their input. |
Awesome. Thanks for the thorough response. This makes a lot of sense, and I think it'd be great to put an exception in QCFractal when protocols are overwritten :-) |
When creating an
OptimizationDataset
, I would expectstore_wavefunction
to get propagated into the generated QCSpecification. However, I don't think this is currently happening -- should it be?Code
Looking at the
qc_specifications
they do seem to be there:It also works for a BasicDataset:
(cc openforcefield/qca-dataset-submission#335 (comment))
Version: 0.50.1
The text was updated successfully, but these errors were encountered: