Skip to content

Conversation

loriab
Copy link
Member

@loriab loriab commented Apr 5, 2022

Description

These are the breaking, user-facing changes for cbs in DDD. That is, these changes aren't needed at present for the cbs back-end, but this is coercing the input front-end into the right syntax.

This is No. 6 of the DDD series, #1351.

Todos

  • All fn(cbs) and passing to extrapolation functions are changed over to fn("cbs") and passing the names of extrapolation functions. This is making cbs behave more like other wrappers: findif you don't call directly, you use kwarg dertype and nbody you don't call directly, you use bsse_type. Functions are trickier to keep imported and don't serialize well, so in ddd, they're registered and replaced by strings, akin to the procedures dictionary.
  • alias functions are replaced with dict specification instead of kwarg specification
  • UpdateHelpers are added so that if you feed an existing input, it prompts you to make the fn -> str changes.
  • Fix bug where allen_focal_point wasn't working b/c used kwargs, not dict, specification for cbs, so the higher deltas were getting lopped off.

Checklist

Status

  • Ready for review
  • Ready for merge

@loriab loriab added the driver For issues that are specifically about Psi's driver. label Apr 5, 2022
@loriab loriab added this to the Psi4 1.6 milestone Apr 5, 2022
Copy link
Contributor

@JonathonMisiewicz JonathonMisiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly documentation requests on my end (no surprise there), but there's a gap in the test suite that needs to be fixed before I can approve this PR.

Copy link
Contributor

@PeterKraus PeterKraus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything here is clear to me and I like the "deprecation" of cbs as a function. It's a good idea to have the "number of steps" variables also for findif and n-body, especially as they're counting the AtomicResults (or whatever it is in DDD). LGTM.

@hokru hokru merged commit 8a91285 into psi4:master Apr 8, 2022
@loriab loriab deleted the easyddd_2 branch April 8, 2022 07:40
zachglick pushed a commit to zachglick/psi4 that referenced this pull request Apr 18, 2022
* new string cbs syntax

* review comments

* testing details

* another win conda attempt
zachglick pushed a commit to zachglick/psi4 that referenced this pull request Apr 18, 2022
* new string cbs syntax

* review comments

* testing details

* another win conda attempt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
driver For issues that are specifically about Psi's driver.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants