BoothGroup / Vayesta

A Python package for wave function-based quantum embedding
Apache License 2.0
34 stars 8 forks source link

Custom solvers as arguments #180

Open obackhouse opened 1 month ago

obackhouse commented 1 month ago

I'd like to be able to pass a custom solver as an argument:

class MyClusterSolver(ClusterSolver):
    def kernel(self, *args, **kwargs):
        # some custom code

dmet = DMET(mf, solver=MyClusterSolver)

As far as I can tell this only supports string identifiers for the built-in solvers. Having a custom solver will be important to me and seems like a nice feature, or is there a workaround?

obackhouse commented 1 month ago

obviously don't mean just for DMET, as a feature of the Embedding base class would be nice

basilib commented 1 month ago

Currently custom solver classes are not supported. However, you can use the callback solver. This allows you to specify a custom function that takes a dummy pyscf MF representing a cluster as an input and returns a results dictionary as output. Currently RDMs, CISD and CCSD amplitudes as well as Green's function moments are automatically recognized and interoperable with vayesta's expectation value calculations. There are some examples for both solids and molecules in the ewf directory. I haven't tested with the DMET module, but it should work with few (or hopefully no changes).

ghb24 commented 1 month ago

Indeed @obackhouse - the callback function that @basilib implemented is how we are currently dealing with external solvers - it seems to work quite well, but open to comments and suggestions. Longer term, it'd be nice to have some data structures for the embedding objects that were more practical to serialize, so that we could also run the calculations entirely separately to vayesta while not retaining the particular instance of the embedding class. However, we can also just dump the information for the cluster hamiltonian to solve externally. I'd suggest having a look at the examples 62-67 in examples/ewf/molecules. Let us know if you need further help.

obackhouse commented 1 month ago

i can check if this will work as a workaround, but in terms of a good interface for users of a vayesta extension, being able to pass the solver itself is the difference between having to do

from myextension import Solver
dmet = DMET(mf, solver=Solver)

and

from myextension import solver_callback
dmet = DMET(mf, solver="CALLBACK", solver_options=dict(callback=solver_callback))

The latter means that you have to access a specific intricacy of the vayesta API to support custom extensions, rather than an intuitive pattern in the former, so I do think that this would be a good feature to support.

maxnus commented 1 month ago

I agree with Ollie's suggestions, but please define an abstract base class which custom solvers need to extend

On Tue, 22 Oct 2024, 08:44 Oliver Backhouse, @.***> wrote:

i can check if this will work as a workaround, but in terms of a good interface for users of a vayesta extension, being able to pass the solver itself is the difference between having to do

from myextension import Solverdmet = DMET(mf, solver=Solver)

and

from myextension import solver_callbackdmet = DMET(mf, solver="CALLBACK", solver_options=dict(callback=solver_callback))

The latter means that you have to access a specific intricacy of the vayesta API to support custom extensions, rather than an intuitive pattern in the former, so I do think that this would be a good feature to support.

— Reply to this email directly, view it on GitHub https://github.com/BoothGroup/Vayesta/issues/180#issuecomment-2428503149, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADBNUNRNX2CD4R477Z3YVL3Z4X66BAVCNFSM6AAAAABQKSRX3SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRYGUYDGMJUHE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

obackhouse commented 1 month ago

I agree with Ollie's suggestions, but please define an abstract base class which custom solvers need to extend

in the absence of static type checking, just using vayesta.solver.solver.ClusterSolver should be fine? kernel is not an abstractmethod but it raises an AbstractMethodError

obackhouse commented 1 month ago

fwiw, callback solvers also don't support custom WaveFunctions, which is another reason to support true custom solvers.

ghb24 commented 1 month ago

Sure - I agree that the custom solver has advantages, and happy to have this in the code alongside the callback approach. Re. the WaveFunction attributes of the solver, I don't think this is strictly needed in general, as long as the desired attributes of the Results object in the solver are stored for the output of the desired system quantities.