QCElemental 0.23.0 update

[jthorton]

Description

This PR should make QCFractal compatible with qcelemental=0.23.0, we now use the json encoding on dicts when adding or updating results which should remove all non-json serializable objects.

Changelog description

Update QCFractal to work with QCElemental=0.23.0

Status

• [x] Code base linted
• [ ] Ready to go

[codecov[bot]]

Codecov Report

Merging #694 (d0567b7) into master (867b1bc) will decrease coverage by 0.02%. The diff coverage is 100.00%.

[jthorton]

It looks like storing the results is working as expected but the dataset view tests seem to have broken, it looks like the results might be a list rather than numpy array? @bennybp any ideas on what's going on here, I would expect the get records to correctly convert the return_result to the correct type? I might be missing a hidden error though.

[bennybp]

Yes, if you can wait for the other features.

This might be a spurious error (that are getting to be a pain in this branch). I will try re-running

[bennybp]

It wasn't a spurious error. The change in qcelemental affects only the properties, not the entire ResultRecord/AtomicResult object.

Does that seem reasonable? Any other modifications?

[jthorton]

@bennybp Thanks for catching this. I am not sure I understand how the properties can be None though the model indicates it should always be a AtomicResultProperties object. Is there a test that checks the properties in the database of a result?

[bennybp]

I think you had the same confusion I did. At that point of the code, the result is a ResultRecord (not an AtomicResult), which has an optional properties. This makes sense because you create a result record before the computation has been run.

(well, it should be optional, but doesn't seem to be, and the default is None): https://github.com/MolSSI/QCFractal/blob/867b1bc9c87ae4be37d9656d95a7bd308b7b79a3/qcfractal/interface/models/records.py#L286-L287

[bennybp]

I have zero idea why this is failing now, and cannot reproduce locally

[bennybp]

Ok seems to be something with alembic on the conda defaults channel not pulling in importlib_resources. The reason for that is a conda-related mystery which I can't be bothered to figure out (since it does seem to be specified as a dependency).

If you install alembic from the default channel, and then do pip install -e . for qcfractal, it then gets installed because pip is smart enough to actually handle dependencies I guess. (but we do --no-deps in the CI)

[bennybp]

Ok ready to merge if you are ok with the psi4 version change

[jthorton]

@bennybp thanks for fixing this, LGTM!