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

Custom solvers as arguments #180

Open
obackhouse opened this issue Oct 21, 2024 · 8 comments
Open

Custom solvers as arguments #180

obackhouse opened this issue Oct 21, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@obackhouse
Copy link
Contributor

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 obackhouse added the enhancement New feature or request label Oct 21, 2024
@obackhouse
Copy link
Contributor Author

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

@basilib
Copy link
Contributor

basilib commented Oct 21, 2024

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
Copy link
Contributor

ghb24 commented Oct 21, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor

maxnus commented Oct 22, 2024 via email

@obackhouse
Copy link
Contributor Author

obackhouse commented Oct 22, 2024

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
Copy link
Contributor Author

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

@ghb24
Copy link
Contributor

ghb24 commented Oct 22, 2024

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.

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

No branches or pull requests

4 participants