Coupled Cluster Rewrite

[amjames]

We (@amjames and @robertodr) propose a partial rewrite of the coupled cluster and DPD functionalities in Psi4. The main motivation is to make the pending PR #1061, Expose wavefunction and amplitudes to python available satisfying the constraints and comments offered by @dgasmith and @amjames. Exposing the T and Lambda amplitudes to the Python layer will have a series of beneficial consequences:

• It will be possible to experiment with these quantities easily.
  • Writing and reading amplitudes to file (HDF5, Zarr, or any other format) is more easily managed from Python, especially if we need to preserve compatibility with Windows.
• Restart capabilities.

  Summary of proposed changes (incomplete)

  cc* modules

  • Create a single cc folder where all the other cc* folders will live as subfolders.
  • Provide a top-level CCWavefunction class, partly modeled on the existing CIWavefunction class to overtake the whole implementation of the CC functionality.
• The class will manage MOInfo,Params, DPD as local data members. This will ensure that the lifetime of these objects is tied to a particular instance of a CC calculation.
• The class will contain the various steps in a CC calculation as methods.
• Move from tstart-tstop to the new timers timer_on-timer_off

  DPD Library

• Remove use of static global machinery from DPD member functions.
• Implement proper copy/move assign/construct for the DPD object.
• Implement internal structs with proper constructors as needed.
• Replace c-style arrays/raw pointers with appropriate smart pointer/containers
• Refactor shared state (shared by multiple DPD instances, currently dpd_gbl struct) into singleton(s).
  • Note: Memory tracking in its current form is not thread safe and relies on the client code properly locking calls to function that need to perform allocations with a mutex.
• DPD will handle opening/closing of psio files instead of requiring the user to do so.

For visibility: @lothian @CDSherrill @andysim @jturney @fevangelista @dgasmith

[CDSherrill]

This sounds nice! Python accessibility of amplitudes would be a great new feature. And cleanup from C structs towards Wavefunction object would also be welcome.

The proposed layer of abstraction between DPD and libpsio is perhaps worth some discussion. libpsio is, itself, a layer of abstraction between Psi and low-level I/O. I think there is an advantage of having a centralized library that all of Psi is supposed to use to do I/O. Then, improvements can be done in only one place (that library), unless there is a design problem with that library's API. The I/O library could be improved to use HDF5, or whatever, on the backend. So, do we need an all-new interface between the CC libraries and libpsio, or is it just that libpsio itself needs its backend replaced?

Best,

David

---

From: Andrew James notifications@github.com Sent: Thursday, September 20, 2018 10:33:45 AM To: psi4/psi4 Cc: Sherrill, David; Mention Subject: [psi4/psi4] Coupled Cluster Rewrite (#1247)

We (@amjameshttps://github.com/amjames and @robertodrhttps://github.com/robertodr) propose a partial rewrite of the coupled cluster and DPD functionalities in Psi4. The main motivation is to make the pending PR #1061, Expose wavefunction and amplitudes to pythonhttps://github.com/psi4/psi4/pull/1061 available satisfying the constraints and comments offered by @dgasmithhttps://github.com/dgasmith and @amjameshttps://github.com/amjames. Exposing the T and Lambda amplitudes to the Python layer will have a series of beneficial consequences:

• It will be possible to experiment with these quantities easily.
• Writing and reading amplitudes to file (HDF5https://portal.hdfgroup.org/display/support, Zarrhttps://zarr.readthedocs.io/en/stable/, or any other format) is more easily managed from Python, especially if we need to preserve compatibility with Windows.
• Restart capabilities.

Summary of proposed changes (incomplete) cc* modules

• Create a single cc folder where all the other cc* folders will live as subfolders.
• Provide a top-level CCWavefunction class, partly modeled on the existing CIWavefunction class to overtake the whole implementation of the CC functionality.
• The class will manage MOInfo,Params, DPD as local data members. This will ensure that the lifetime of these objects is tied to a particular instance of a CC calculation.
• The class will contain the various steps in a CC calculation as methods.
• Move from tstart-tstop to the new timers timer_on-timer_off

DPD Library

• Remove use of static global machinery from DPD member functions.
• Implement proper copy/move assign/construct for the DPD object.
• Implement internal structs with proper constructors as needed.
• Replace c-style arrays/raw pointers with appropriate smart pointer/containers
• Refactor shared state (shared by multiple DPD instances, currently dpd_gbl struct) into singleton(s).
  • Note: Memory tracking in its current form is not thread safe and relies on the client code properly locking calls to function that need to perform allocations with a mutex.
• Insert a layer of abstraction between DPD and psio. (slated for deprecation/replacement)

For visibility: @lothianhttps://github.com/lothian @CDSherrillhttps://github.com/CDSherrill @andysimhttps://github.com/andysim @jturneyhttps://github.com/jturney @fevangelistahttps://github.com/fevangelista @dgasmithhttps://github.com/dgasmith

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/psi4/psi4/issues/1247, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AC9Qdt-9Cic7AcHOMo2bMq1iF3Nyc5FZks5uc6dJgaJpZM4WyQ3-.

[amjames]

@CDSherrill I was under the impression that psio was going to be completely removed, but if the plan is to replace the internals and keep psio around as the abstraction layer then there is no need for DPD to implement one.

However we do need DPD to insulate its user. Since the DPD instances rely on files being open/closed externally any other code using any DPD instance that does not properly ensure files are managed will leave all DPD instances in an invalid state. This is especially important for exposing amplitude access py side since in theory those accessor methods could be called at any point after a handle to the ccwfn is obtained.

I guess what I really meant by that point wasn't that something needs to go in between DPD <-> PSIO, rather that DPD shouldn't be passing responsibility over management of an implementation detail on to it's users.

[CDSherrill]

@amjames, to my knowledge there is not yet a detailed plan about I/O ... your comments gave me an excuse to bring up the topic prior to our next developers' meeting.

What you're saying about DPD makes sense to me... the users of DPD shouldn't have to worry about opening/closing files themselves, DPD should handle these details for them. I didn't realize or remember that it didn't already handle this.

---

From: Andrew James notifications@github.com Sent: Thursday, September 20, 2018 2:07:40 PM To: psi4/psi4 Cc: Sherrill, David; Mention Subject: Re: [psi4/psi4] Coupled Cluster Rewrite (#1247)

@CDSherrillhttps://github.com/CDSherrill I was under the impression that psio was going to be completely removed, but if the plan is to replace the internals and keep psio around as the abstraction layer then there is no need for DPD to implement one.

However we do need DPD to insulate its user. Since the DPD instances rely on files being open/closed externally any other code using any DPD instance that does not properly ensure files are managed will leave all DPD instances in an invalid state. This is especially important for exposing amplitude access py side since in theory those accessor methods could be called at any point after a handle to the ccwfn is obtained.

I guess what I really meant by that point wasn't that something needs to go in between DPD <-> PSIO, rather that DPD shouldn't be passing responsibility over management of an implementation detail on to it's users.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/psi4/psi4/issues/1247#issuecomment-423279853, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AC9QdoFCG1HVF9-VktlmDeetttgktmX6ks5uc9lsgaJpZM4WyQ3-.

[tomspur]

Thanks @amjames for working on this! Let me know, if I can help further with this to get the python and restart features into psi.