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

Simplified solver interface for adding new solvers #82

Merged
merged 10 commits into from
Aug 20, 2024

Conversation

alberto-carta
Copy link
Collaborator

@alberto-carta alberto-carta commented Aug 9, 2024

Main changes

Broke up the SolverStructure object in multiple classes

  • dmft_tools/solver.py now contains only a dispatcher create_solver that imports the corresponding modules if it finds them and points to the correct DMFT solver class
  • there is a new module folder solvers which contains the abstract class and every child class
  • dmft_tools/solvers/abstractdmftsolver.py contains an abstract class AbstractDMFTSolver that cannot be initialized, but contains all the common parts of the init of each solver as well as some commonly used functions (I kept the tail fitting here even though we use it basically only once, but I think in theory it should be usable with every imaginary freq solver)
  • each solver gets a module dmft_tools/solvers/SOLVERNAME_interface.py which contains the SOLVERNAMEInterface class which always inherits from AbstractDMFTSolver and adds the solver specifics, I have made sure alway to keep separate the triqs_solver object from the solid_dmft solver object (there were problems for ctint with some of the flags not being passed correctly)

Minor changes

  • Hartree solver now saves the correct G_iw
  • Fixed Hartree solver tests
  • "get_n_orbitals" from dmft_tools/solver.py is moved to a new module dmft_tools/common.py
  • deprecated inchworm interface

I tested the implementations with all solvers we have and with the solid_dmft tests.

This should streamline adding new solvers for the future

Copy link
Member

@the-hampel the-hampel left a comment

Choose a reason for hiding this comment

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

Hi @alberto-carta ,

these changes are great! Using an abstract solver class and having all solvers live in there separate solver modules makes everything much easier to find and easier to maintain. It seems that all solvers are working. I did not make any further tests so we will keep an eye on these changes in unstable.

The only things I did in two commits are:

  • clean up imports
  • make the error message more verbose when the solver cannot be imported. This can have two causes right? Either the user misspelled the name of the solver or the solver is actually not installed. Correct me if I am wrong. So I just made the print more explicit.
  • I run pep8 auto format. I wanted to do this for a long time on the solver class but did not dare to. Now is a good time to enforce the formatting.

@alberto-carta alberto-carta merged commit c642030 into unstable Aug 20, 2024
2 checks passed
@alberto-carta alberto-carta deleted the solver_interface_refactoring branch August 20, 2024 12:55
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 this pull request may close these issues.

2 participants