MolSSI / QCFractal

A distributed compute and database platform for quantum chemistry.
https://molssi.github.io/QCFractal/
BSD 3-Clause "New" or "Revised" License
144 stars 47 forks source link

Hack fix for d3 calculations which shouldn't be split in the planner #691

Closed bennybp closed 2 years ago

bennybp commented 2 years ago

Description

Fixes issue #688

These functionals shouldn't be split in the composition planner, as the first part isn't available in psi4 as a plain functional.

This is a temporary fix. The next major version will probably do away with the composition planner altogether.

Changelog description

Skip composition planner for certain functionals

Status

codecov[bot] commented 2 years ago

Codecov Report

Merging #691 (649d387) into master (8a291bc) will decrease coverage by 0.03%. The diff coverage is 50.00%.

dotsdl commented 2 years ago

Doesn't this change only impact how the records are accessed? From the op it looks like the computation has been split among tasks, with errors occurring at compute time on the manager under psi4.

bennybp commented 2 years ago

The composition planner is called both for computing and accessing:

https://github.com/MolSSI/QCFractal/blob/8a291bc5b9063d474045c712eb4b4569acf871a3/qcfractal/interface/collections/dataset.py#L1136

https://github.com/MolSSI/QCFractal/blob/8a291bc5b9063d474045c712eb4b4569acf871a3/qcfractal/interface/collections/dataset.py#L1223

I guess there may still be a problem with existing records? But with the way datasets are currently written that shouldn't be an issue. The previously submitted ones will disappear from the dataset (because the dataset doesn't explicitly link to individual records)

dotsdl commented 2 years ago

The composition planner is called both for computing and accessing:

https://github.com/MolSSI/QCFractal/blob/8a291bc5b9063d474045c712eb4b4569acf871a3/qcfractal/interface/collections/dataset.py#L1136

https://github.com/MolSSI/QCFractal/blob/8a291bc5b9063d474045c712eb4b4569acf871a3/qcfractal/interface/collections/dataset.py#L1223

I guess there may still be a problem with existing records? But with the way datasets are currently written that shouldn't be an issue. The previously submitted ones will disappear from the dataset (because the dataset doesn't explicitly link to individual records)

Ah perfect, you're right, missed the usage in Dataset._compute. With this change use of a new qcportal from a new QCFractal release should generate the right set of records when called. Can basically try to resubmit the dataset and this should just work.