Skip to content

Commit

Permalink
fix the rest, incl. let dicts be cast to models based on schema_versi…
Browse files Browse the repository at this point in the history
…on field if present
  • Loading branch information
loriab committed Nov 5, 2024
1 parent 0d77110 commit 32d9b24
Show file tree
Hide file tree
Showing 14 changed files with 105 additions and 47 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,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@sse_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
1 change: 1 addition & 0 deletions docs/source/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ 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``.

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
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
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
2 changes: 1 addition & 1 deletion qcengine/programs/psi4.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from typing import TYPE_CHECKING, Any, ClassVar, Dict

from qcelemental.models.v1 import AtomicResult as AtomicResult
from qcelemental.models.v2 import AtomicResult as BasisSet
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
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
3 changes: 1 addition & 2 deletions qcengine/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,7 @@ def compute(self, input_data: "AtomicInput", config: "TaskConfig") -> "AtomicRes
grad = [0, 0, -grad_value, 0, 0, grad_value]

if mode == "pass":
# TODO return v2 schema_versions[2].AtomicResult(
return qcel.models.v1.AtomicResult(
return schema_versions[2].AtomicResult(
**{
**input_data.dict(),
**{
Expand Down
31 changes: 17 additions & 14 deletions qcengine/tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import qcelemental

from qcengine import cli, get_molecule, util
from qcengine.testing import checkver_and_convert, schema_versions2, using
from qcengine.testing import checkver_and_convert, schema_versions, using


def run_qcengine_cli(args: List[str], stdin: str = None) -> str:
Expand Down Expand Up @@ -57,9 +57,9 @@ def test_info():


@using("psi4")
def test_run_psi4(tmp_path, schema_versions2, request):
def test_run_psi4(tmp_path, schema_versions, request):
"""Tests qcengine run with psi4 and JSON input"""
models, _, _ = schema_versions2
models, retver, _ = schema_versions

def check_result(stdout):
output = json.loads(stdout)
Expand All @@ -74,14 +74,14 @@ def check_result(stdout):
)

inp = checkver_and_convert(inp, request.node.name, "pre")
args = ["run", "psi4", inp.model_dump_json()]
args = ["run", "psi4", f"--return-version={retver}", inp.model_dump_json()]
check_result(run_qcengine_cli(args))

args = ["run", "psi4", os.path.join(tmp_path, "input.json")]
args = ["run", "psi4", os.path.join(tmp_path, "input.json"), f"--return-version={retver}"]
with util.disk_files({"input.json": inp.model_dump_json()}, {}, cwd=tmp_path):
check_result(run_qcengine_cli(args))

args = ["run", "psi4", "-"]
args = ["run", "psi4", f"--return-version={retver}", "-"]
# model_dump_json() works on v1 or v2 (see above). below tests that json() still works on v1.
if "as_v1" in request.node.name:
check_result(run_qcengine_cli(args, stdin=inp.json()))
Expand All @@ -91,9 +91,9 @@ def check_result(stdout):

@using("geometric")
@using("psi4")
def test_run_procedure(tmp_path, schema_versions2, request):
def test_run_procedure(tmp_path, schema_versions, request):
"""Tests qcengine run-procedure with geometric, psi4, and JSON input"""
models, _, _ = schema_versions2
models, retver, _ = schema_versions

def check_result(stdout):
output = json.loads(stdout)
Expand All @@ -109,16 +109,19 @@ def check_result(stdout):
inp = models.OptimizationInput(**inp)

inp = checkver_and_convert(inp, request.node.name, "pre")
args = ["run-procedure", "geometric", inp.model_dump_json()]
if "to_v" in request.node.name:
args = ["run", "geometric", inp.model_dump_json(), f"--return-version={retver}"]
else:
args = ["run-procedure", "geometric", inp.model_dump_json()]
check_result(run_qcengine_cli(args))

args = ["run-procedure", "geometric", os.path.join(tmp_path, "input.json")]
if "to_v" in request.node.name:
args = ["run", "geometric", f"--return-version={retver}", os.path.join(tmp_path, "input.json")]
else:
args = ["run-procedure", "geometric", os.path.join(tmp_path, "input.json")]
with util.disk_files({"input.json": inp.model_dump_json()}, {}, cwd=tmp_path):
check_result(run_qcengine_cli(args))

args = ["run-procedure", "geometric", inp.model_dump_json()]
check_result(run_qcengine_cli(args, stdin=inp.model_dump_json()))

# try unified route
args = ["run", "geometric", inp.model_dump_json()]
args = ["run", "geometric", inp.model_dump_json(), f"--return-version={retver}"]
check_result(run_qcengine_cli(args, stdin=inp.model_dump_json()))
4 changes: 3 additions & 1 deletion qcengine/tests/test_procedures.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,8 @@ def test_torsiondrive_generic(schema_versions, request):
ret = qcng.compute(input_data, "torsiondrive", raise_error=True, return_version=retver)
ret = checkver_and_convert(ret, request.node.name, "post")

assert ret.error is None
if "_v2" not in request.node.name:
assert ret.error is None
assert ret.success

expected_grid_ids = {"180", "0"}
Expand All @@ -422,6 +423,7 @@ def test_torsiondrive_generic(schema_versions, request):
assert ret.provenance.creator.lower() == "torsiondrive"
assert ret.optimization_history["180"][0].provenance.creator.lower() == "geometric"
assert ret.optimization_history["180"][0].trajectory[0].provenance.creator.lower() == "rdkit"
assert ret.optimization_history["180"][0].trajectory[0].schema_version == 2 if ("_v2" in request.node.name) else 1

assert ret.stdout == "All optimizations converged at lowest energy. Job Finished!\n"

Expand Down
17 changes: 11 additions & 6 deletions qcengine/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,15 +226,20 @@ def handle_output_metadata(
}[convert_version]

# for input_data, use object (not dict) if possible for >=v2
ret = model(
success=output_fusion.pop("success", False),
error=output_fusion.pop("error"),
input_data=output_data if (convert_version >= 2) else output_fusion,
)
success_ret = output_fusion.pop("success", False)
error_ret = output_fusion.pop("error")
if convert_version >= 2:
if isinstance(output_data, (qcelemental.models.v1.FailedOperation, qcelemental.models.v2.FailedOperation)):
# when harnesses return FailedOp object rather than raising error
inp_ret = output_data.input_data
else:
inp_ret = output_data.__class__(**output_fusion)
else:
inp_ret = output_fusion
ret = model(success=success_ret, error=error_ret, input_data=inp_ret)

if convert_version > 0:
ret = ret.convert_v(convert_version)
returned_version = getattr(ret, "schema_version", "not a model")

if return_dict:
# Use Pydantic to serialize, then reconstruct as Python dict of Python Primals
Expand Down

0 comments on commit 32d9b24

Please sign in to comment.