Skip to content
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

[SUGGESTION] berny procedure engine specification change #302

Open
coltonbh opened this issue May 19, 2021 · 1 comment · May be fixed by #303
Open

[SUGGESTION] berny procedure engine specification change #302

coltonbh opened this issue May 19, 2021 · 1 comment · May be fixed by #303

Comments

@coltonbh
Copy link
Contributor

coltonbh commented May 19, 2021

Is your feature request related to a problem? Please describe.

The BernyProcedure currently gets the compute engine to use from the keywords dictionary used to define keywords for the optimization procedure. I think it would be cleaner to get the engine value from the specification of the compute model instead. This would keep all keywords related to the compute engine in one dictionary, and all keywords related to the optimization in the other keywords dictionary.

The way it currently is

water = Molecule.from_data("pubchem:water")

input_spec = QCInputSpecification(
    driver="gradient",
    model={"method": "b3lyp", "basis": "6-31g"},
    # Keywords for compute engine
    keywords={"psi4keyword": "psi4value"},
)

opt_input = OptimizationInput(
    initial_molecule=water,
    input_specification=input_spec,
    protocols={"trajectory": "all"},
    # Define compute engine and keywords for pyberny
    # This seems an odd combination of keywords
    keywords={"program": "psi4", "bernykeyword": "bernyvalue"},
)

qcng.compute_procedure(opt_input, 'berny')

Then this line gets the "program" keyword to get a compute engine.

Describe the solution you'd like

water = Molecule.from_data("pubchem:water")

input_spec = QCInputSpecification(
    driver="gradient",
    model={"method": "b3lyp", "basis": "6-31g"},
    # QC program and its associated keywords kept together
    keywords={"program": "psi4", "psi4keyword": "psi4value"},
)

opt_input = OptimizationInput(
    initial_molecule=water,
    input_specification=input_spec,
    protocols={"trajectory": "all"},
    # Don't mix the program definition with keywords specific to the optimizer
    keywords={"bernykeyword1": "bernyvalue1"},
)

qcng.compute_procedure(opt_input, 'berny')

Change this line as follows:

Current:

program = input_data["keywords"].pop("program")

Desired:

program = comput["keywords"].pop("program")

Since geometric uses a different (internal) mechanism for accessing compute engines, this change would only affect the BernyProcedure

@coltonbh
Copy link
Contributor Author

Actually, perhaps the confusion (for me) around where the "program" is defined would be better addressed by this proposal instead, which seeks to improve a few of the issues I came across while implementing various procedures. MolSSI/QCElemental#264

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant