Skip to content

Commit

Permalink
Csse layout 550 error (#458)
Browse files Browse the repository at this point in the history
* failedoperation.input_data as object when possible

* fix more tests, incl. d3/d4 return FailedOp and RandomError treated same as others

* fix the rest, incl. let dicts be cast to models based on schema_version field if present

* fix xtb

* fixes for py38 and dftd3

* fix for adcc

* test dftd3 on windows

* try windows and new d3

* fix

* test path

* regular testing lanes
  • Loading branch information
loriab committed Jan 17, 2025
1 parent 614b77c commit f2b2a17
Show file tree
Hide file tree
Showing 22 changed files with 285 additions and 61 deletions.
9 changes: 8 additions & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ jobs:
runs-on: windows-latest
pytest: "-k 'not (hes2 or qchem)'"

# 3.13
#- conda-env: disp-win
# python-version: 3.12
# label: disp-win
# runs-on: windows-latest
# pytest: ""

- conda-env: mace
python-version: "3.10"
label: MACE
Expand Down Expand Up @@ -171,7 +178,7 @@ jobs:
#if: false
run: |
conda remove qcelemental --force
python -m pip install 'git+https://github.com/loriab/QCElemental.git@csse_pyd2_warnings' --no-deps
python -m pip install 'git+https://github.com/loriab/QCElemental.git@csse_layout_536a' --no-deps
# note: conda remove --force, not mamba remove --force b/c https://github.com/mamba-org/mamba/issues/412
# alt. is micromamba but not yet ready for setup-miniconda https://github.com/conda-incubator/setup-miniconda/issues/75
Expand Down
26 changes: 26 additions & 0 deletions devtools/conda-envs/disp-win.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: test
channels:
- conda-forge
- nodefaults
dependencies:
- dftd3-python =1.2.0
- gcp-correction
- geometric
- optking
- pymdi

# Core
- python
- pyyaml
- py-cpuinfo
- psutil
- qcelemental
- pydantic=2
- pydantic-settings
- msgpack-python

# Testing
- pytest
- pytest-cov
- codecov

5 changes: 5 additions & 0 deletions docs/source/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ Misc.

MUST (Unmerged)
+++++++++++++++
- When qcengine.compute() fails and forms a fop = FailedOperation (raise_error=T), with v2, `fop.input_data` will be an <>Input model (when possible; if the error was in forming the model, it'll still be a dict), not always a dict like v1.
- When <executor>.compute() fails and collects the input for processing, with v2 it now uses the <>Input model passed to the executor, not the model-or-dict passed into compute().
- The net result of the two above is that whereas fop.input_data in v1 was reliably a dict and its contents would reflect whether a model or dict was passed to qcengine.compute(), now in v2, fop.input_data is a model whenever possible (to mirror <>Result.input_data) regardless of model or dict passed to qcengine.compute(); the only case where it's a dict is if the error was in forming the model.
- DFTD3 & DFTD4 (new intf) - intercept ``v1.AtomicResult`` with ``success=False`` and ``error`` fields set from QCSchema interfaces and return ``FailedOperation``s. Someday when upstream switches to v2, request packages return FaileOp directly and use ``input_error`` rather than ``input error``.
- ``qcengine run`` learned new argument ``--return-version`` analogous to ``qcengine.compute(..., return_version=1|2)`` so CLI matches API capabilities. Note *not* ported to phasing-out ``qcengine run-procedure``.

WIP (Unmerged)
++++++++++++++
Expand Down
10 changes: 9 additions & 1 deletion qcengine/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ def parse_args():
"(ii) A file name, "
"(iii) '-', indicating data will be read from STDIN.",
)
run.add_argument(
"--return-version", default=-1, type=int, help="Schema version to return, if not the input version"
)

run_procedure = subparsers.add_parser(
"run-procedure",
Expand Down Expand Up @@ -214,7 +217,12 @@ def main(args=None):
if command == "info":
info_cli(args)
elif command == "run":
ret = compute(data_arg_helper(args["data"]), args["program"], task_config=task_config)
ret = compute(
data_arg_helper(args["data"]),
args["program"],
task_config=task_config,
return_version=args["return_version"],
)
print(ret.json())
elif command == "run-procedure":
ret = compute_procedure(data_arg_helper(args["data"]), args["procedure"], task_config=task_config)
Expand Down
26 changes: 26 additions & 0 deletions qcengine/compute.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,28 @@ def compute(
AtomicResult, OptimizationResult, FailedOperation, etc., or Dict representation of any object type
A QCSchema representation of the requested output, type depends on return_dict key.
.. _`table:compute_result`:
+-----------+-------------+-------------+--------------------------------------------------+
| good_calc | raise_error | return_dict | output |
+===========+=============+=============+==================================================+
| T | F (def) | F (def) | ``AtomicResult`` object |
+-----------+-------------+-------------+--------------------------------------------------+
| T | T | F (def) | ``AtomicResult`` object |
+-----------+-------------+-------------+--------------------------------------------------+
| T | T | T | dict of ``AtomicResult`` |
+-----------+-------------+-------------+--------------------------------------------------+
| T | F (def) | T | dict of ``AtomicResult`` |
+-----------+-------------+-------------+--------------------------------------------------+
| F | F (def) | F (def) | ``FailedOperation`` object |
+-----------+-------------+-------------+--------------------------------------------------+
| F | T | F (def) | raises ``InputError`` (type encoded in FailedOp) |
+-----------+-------------+-------------+--------------------------------------------------+
| F | T | T | raises ``InputError`` (type encoded in FailedOp) |
+-----------+-------------+-------------+--------------------------------------------------+
| F | F (def) | T | dict of ``FailedOperation`` |
+-----------+-------------+-------------+--------------------------------------------------+
.. versionadded:: 0.50.0
input_data can newly be QCSchema v2 as well as longstanding v1.
The compute_procedure is newly incorporated into compute.
Expand Down Expand Up @@ -113,12 +135,16 @@ def compute(
output_data = executor.compute(input_data, config)
break
except RandomError as e:
if return_version >= 2:
output_data = input_data

if x == config.retries:
raise e
else:
metadata["retries"] += 1
except:
if return_version >= 2:
output_data = input_data
raise

return handle_output_metadata(
Expand Down
6 changes: 4 additions & 2 deletions qcengine/procedures/berny.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
from typing import Any, ClassVar, Dict, Union

import numpy as np
from qcelemental.models.v2 import FailedOperation, OptimizationInput, OptimizationResult
from qcelemental.models.v1 import OptimizationResult
from qcelemental.models.v2 import FailedOperation, OptimizationInput
from qcelemental.util import which_import

import qcengine
Expand Down Expand Up @@ -94,5 +95,6 @@ def compute(
"stdout": log_stream.getvalue(), # collect logged messages
}
)
return OptimizationResult(**output_data)
output_data = OptimizationResult(**output_data)
return output_data.convert_v(2)
return FailedOperation(input_data=input_data, error=error)
4 changes: 3 additions & 1 deletion qcengine/procedures/geometric.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Any, ClassVar, Dict, Union

from qcelemental.models.v2 import OptimizationInput, OptimizationResult
from qcelemental.models.v1 import OptimizationResult
from qcelemental.models.v2 import OptimizationInput
from qcelemental.util import safe_version, which_import

from .model import ProcedureHarness
Expand Down Expand Up @@ -65,5 +66,6 @@ def compute(self, input_model: "OptimizationInput", config: "TaskConfig") -> "Op
output_data["input_specification"]["extras"].pop("_qcengine_local_config", None)
if output_data["success"]:
output_data = OptimizationResult(**output_data)
output_data = output_data.convert_v(2)

return output_data
12 changes: 9 additions & 3 deletions qcengine/procedures/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,15 @@ def _build_model(
# remember these are user-provided dictionaries, so they'll have the mandatory fields,
# like driver, not the helpful discriminator fields like schema_version.

# for now, the two dictionaries look the same, so cast to the one we want
# note that this prevents correctly identifying the user schema version when dict passed in, so either as_v1/None or as_v2 will fail
mdl = model_wrapper(data, v1_model)
schver = data.get("schema_version")
if schver == 1:
mdl = model_wrapper(data, v1_model)
elif schver == 2:
mdl = model_wrapper(data, v2_model)
else:
# for now, the two dictionaries look the same, so cast to the one we want
# note that this prevents correctly identifying the user schema version when dict passed in, so either as_v1/None or as_v2 will fail
mdl = model_wrapper(data, v1_model)

input_schema_version = mdl.schema_version
if return_input_schema_version:
Expand Down
4 changes: 3 additions & 1 deletion qcengine/procedures/optking.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Any, ClassVar, Dict, Union

from qcelemental.models.v2 import OptimizationInput, OptimizationResult
from qcelemental.models.v1 import OptimizationResult
from qcelemental.models.v2 import OptimizationInput
from qcelemental.util import safe_version, which_import

from .model import ProcedureHarness
Expand Down Expand Up @@ -55,5 +56,6 @@ def compute(self, input_model: "OptimizationInput", config: "TaskConfig") -> "Op
output_data["input_specification"]["extras"].pop("_qcengine_local_config", None)
if output_data["success"]:
output_data = OptimizationResult(**output_data)
output_data = output_data.convert_v(2)

return output_data
12 changes: 11 additions & 1 deletion qcengine/programs/dftd_ng.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from typing import Any, ClassVar, Dict

import qcelemental
from qcelemental.models.v2 import AtomicInput, AtomicResult
from qcelemental.models.v2 import AtomicInput, AtomicResult, FailedOperation
from qcelemental.util import parse_version, safe_version, which_import

from ..config import TaskConfig
Expand Down Expand Up @@ -109,6 +109,11 @@ def compute(self, input_model: AtomicInput, config: TaskConfig) -> AtomicResult:

# Run the Harness
output = run_qcschema(input_model)

# d4 qcschema interface stores error in Result model
if not output.success:
return FailedOperation(input_data=input_data, error=output.error.model_dump())

output = output.convert_v(2)

if "info" in output.extras:
Expand Down Expand Up @@ -278,6 +283,11 @@ def compute(self, input_model: AtomicInput, config: TaskConfig) -> AtomicResult:

# Run the Harness
output = run_qcschema(input_model)

# d3 qcschema interface stores error in Result model
if not output.success:
return FailedOperation(input_data=input_data, error=output.error.model_dump())

output = output.convert_v(2)

if "info" in output.extras:
Expand Down
26 changes: 17 additions & 9 deletions qcengine/programs/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,19 +76,27 @@ def build_input_model(
# Note: Someday when the multiple QCSchema versions QCEngine supports are all within the
# Pydantic v2 API base class, this can use discriminated unions instead of logic.

if isinstance(data, qcelemental.models.v1.AtomicInput):
mdl = model_wrapper(data, qcelemental.models.v1.AtomicInput)
elif isinstance(data, qcelemental.models.v2.AtomicInput):
mdl = model_wrapper(data, qcelemental.models.v2.AtomicInput)
v1_model = getattr(qcelemental.models.v1, "AtomicInput")
v2_model = getattr(qcelemental.models.v2, "AtomicInput")

if isinstance(data, v1_model):
mdl = model_wrapper(data, v1_model)
elif isinstance(data, v2_model):
mdl = model_wrapper(data, v2_model)
elif isinstance(data, dict):
# remember these are user-provided dictionaries, so they'll have the mandatory fields,
# like driver, not the helpful discriminator fields like schema_version.

# for now, the two dictionaries look the same, so cast to the one we want
# note that this prevents correctly identifying the user schema version when dict passed in, so either as_v1/None or as_v2 will fail
mdl = model_wrapper(
data, qcelemental.models.v1.AtomicInput
) # TODO v2 # TODO kill off excuse_as_v2, now fix 2->-1 in schema_versions
schver = data.get("schema_version")
if schver == 1:
mdl = model_wrapper(data, v1_model)
elif schver == 2:
mdl = model_wrapper(data, v2_model)
else:
# for now, the two dictionaries look the same, so cast to the one we want
# note that this prevents correctly identifying the user schema version when dict passed in, so either as_v1/None or as_v2 will fail
# TODO v2 # TODO kill off excuse_as_v2, now fix 2->-1 in schema_versions
mdl = model_wrapper(data, v1_model)

input_schema_version = mdl.schema_version
if return_input_schema_version:
Expand Down
6 changes: 4 additions & 2 deletions qcengine/programs/psi4.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
from pathlib import Path
from typing import TYPE_CHECKING, Any, ClassVar, Dict

from qcelemental.models.v2 import AtomicResult, BasisSet
from qcelemental.models.v1 import AtomicResult as AtomicResult
from qcelemental.models.v2 import BasisSet
from qcelemental.util import deserialize, parse_version, safe_version, which, which_import

from ..exceptions import InputError, RandomError, ResourceError, UnknownError
Expand Down Expand Up @@ -306,4 +307,5 @@ def compute(self, input_model: "AtomicInput", config: "TaskConfig") -> "AtomicRe
# Delete keys
output_data.pop("return_output", None)

return AtomicResult(**output_data)
atres = AtomicResult(**output_data)
return atres.convert_v(2)
4 changes: 3 additions & 1 deletion qcengine/programs/tests/test_molpro.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ def test_molpro_output_parser(test_case):
data = molpro_info.get_test_data(test_case)
inp = qcel.models.v1.AtomicInput.parse_raw(data["input.json"])

output = qcng.get_program("molpro", check=False).parse_output(data, inp).dict()
output = qcng.get_program("molpro", check=False).parse_output(data, inp)
# only qcng.compute() handles schema versions. above returns v2, so need to convert
output = output.convert_v(1).dict()
output.pop("provenance", None)
output.pop("schema_version", None)

Expand Down
30 changes: 25 additions & 5 deletions qcengine/programs/tests/test_programs.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,22 +255,36 @@ def test_mopac_task(schema_versions, request):
assert "== MOPAC DONE ==" in ret.stdout


def test_random_failure_no_retries(failure_engine, schema_versions, request):
def test_random_failure_no_retries_input(failure_engine, schema_versions, request):
_, retver, _ = schema_versions

failure_engine.iter_modes = ["input_error"]
ret = qcng.compute(failure_engine.get_job(), failure_engine.name, raise_error=False, return_version=retver)
ret = checkver_and_convert(ret, request.node.name, "post", vercheck=False)

assert ret.error.error_type == "input_error"
assert "retries" not in ret.input_data["provenance"].keys()
provenance_keys = (
ret.input_data.provenance.model_dump().keys()
if ("_v2" in request.node.name)
else ret.input_data["provenance"].keys()
)
assert "retries" not in provenance_keys


def test_random_failure_no_retries_random(failure_engine, schema_versions, request):
_, retver, _ = schema_versions

failure_engine.iter_modes = ["random_error"]
ret = qcng.compute(failure_engine.get_job(), failure_engine.name, raise_error=False, return_version=retver)
ret = checkver_and_convert(ret, request.node.name, "post", vercheck=False)

assert ret.error.error_type == "random_error"
assert "retries" not in ret.input_data["provenance"].keys()
provenance_keys = (
ret.input_data.provenance.model_dump().keys()
if ("_v2" in request.node.name)
else ret.input_data["provenance"].keys()
)
assert "retries" not in provenance_keys


def test_random_failure_with_retries(failure_engine, schema_versions, request):
Expand All @@ -286,7 +300,10 @@ def test_random_failure_with_retries(failure_engine, schema_versions, request):
)
ret = checkver_and_convert(ret, request.node.name, "post", vercheck=False)

assert ret.input_data["provenance"]["retries"] == 2
retries = (
ret.input_data.provenance.retries if ("_v2" in request.node.name) else ret.input_data["provenance"]["retries"]
)
assert retries == 2
assert ret.error.error_type == "random_error"

failure_engine.iter_modes = ["random_error", "input_error"]
Expand All @@ -299,7 +316,10 @@ def test_random_failure_with_retries(failure_engine, schema_versions, request):
)
ret = checkver_and_convert(ret, request.node.name, "post", vercheck=False)

assert ret.input_data["provenance"]["retries"] == 1
retries = (
ret.input_data.provenance.retries if ("_v2" in request.node.name) else ret.input_data["provenance"]["retries"]
)
assert retries == 1
assert ret.error.error_type == "input_error"


Expand Down
12 changes: 9 additions & 3 deletions qcengine/programs/tests/test_qchem.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ def test_qchem_output_parser(test_case):
inp = qcel.models.v1.AtomicInput.parse_raw(data["input.json"])

outfiles = qcel.util.deserialize(data["outfiles.msgpack"], "msgpack-ext")
output = qcng.get_program("qchem", check=False).parse_output(outfiles, inp).dict()
output = qcng.get_program("qchem", check=False).parse_output(outfiles, inp)
# only qcng.compute() handles schema versions. above returns v2, so need to convert
output = output.convert_v(1).dict()
output.pop("provenance", None)

output_ref = qcel.models.v1.AtomicResult.parse_raw(data["output.json"]).dict()
Expand Down Expand Up @@ -129,7 +131,9 @@ def test_qchem_logfile_parser(test_case):
data = qchem_logonly_info.get_test_data(test_case)
outfiles = {"dispatch.out": data["qchem.out"]}
with pytest.warns(Warning):
output = qcng.get_program("qchem", check=False).parse_logfile(outfiles).dict()
output = qcng.get_program("qchem", check=False).parse_logfile(outfiles)
# only qcng.compute() handles schema versions. above returns v2, so need to convert
output = output.convert_v(1).dict()
output["stdout"] = None

output_ref = qcel.models.v1.AtomicResult.parse_raw(data["output.json"]).dict()
Expand All @@ -152,7 +156,9 @@ def test_qchem_logfile_parser_qcscr(test_case):
outfiles = qcel.util.deserialize(data["outfiles.msgpack"], "msgpack-ext")

with pytest.warns(Warning):
output = qcng.get_program("qchem", check=False).parse_logfile(outfiles).dict()
output = qcng.get_program("qchem", check=False).parse_logfile(outfiles)
# only qcng.compute() handles schema versions. above returns v2, so need to convert
output = output.convert_v(1).dict()
output["stdout"] = None

output_ref = qcel.models.v1.AtomicResult.parse_raw(data["output.json"]).dict()
Expand Down
Loading

0 comments on commit f2b2a17

Please sign in to comment.