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

HubbardHamiltonian as child class of sisl.Hamiltonian? #119

Open
tfrederiksen opened this issue Feb 23, 2022 · 5 comments
Open

HubbardHamiltonian as child class of sisl.Hamiltonian? #119

tfrederiksen opened this issue Feb 23, 2022 · 5 comments

Comments

@tfrederiksen
Copy link
Member

Wouldn't it not be advantageous to change class HubbardHamiltonian(object) to be a child class of sisl.Hamiltonian? Now it seems to me more logical and we could avoid duplicating a lot of stuff.

In fact, perhaps we should think of an intermediate class SCFHamiltonian(sisl.Hamiltonian) that would generalize things for iterative/SCF problems (it could perhaps belong in sisl?), and then class HubbardHamiltonian(SCFHamiltonian) for the specifics of our MFH implementation?

@zerothi
Copy link
Collaborator

zerothi commented Feb 23, 2022

I think an SCFHamiltonian is the way forward, I think SCF objects should hold the objects that they act on. The problem is that Hamiltonian should do 1 thing, and that is interacting with the Hamiltonian. This is why I deprecated Hamiltonian.DOS etc. it just doesn't make sense. Bloating a class to do tons of different things makes it hard to maintain...

I have this issue, zerothi/sisl#97.
That would be ideal, IMHO.

@tfrederiksen
Copy link
Member Author

Thanks, I had forgotten about zerothi/sisl#97. Any particular inheritance in such a SCFHamiltonian class? Or is it rather a new one from scratch? (Not sure I understand what you mean by holding the objects they act on).

@zerothi
Copy link
Collaborator

zerothi commented Feb 23, 2022

Thanks, I had forgotten about zerothi/sisl#97. Any particular inheritance in such a SCFHamiltonian class? Or is it rather a new one from scratch? (Not sure I understand what you mean by holding the objects they act on).

I would do something like this:

class SCF:
    # implement however this one does it

scf = SCF(HubbardHamiltonian, *options_for_scf_or_whatever)
scf.iterate()

I.e. SCF stuff should not belong to the Hamiltonian, in my opinion.

@tfrederiksen
Copy link
Member Author

tfrederiksen commented Feb 23, 2022

Thanks, I think I grasp your idea. In our specific case it could be something like

class SCF:
    # general class

class MFH(SCF):
    # child class with specifics for the mean-field Hubbard model

mfh = MFH(h0, dm0, U=3, *options)
h1, dm1 = mfh.iterate()
h_conv, dm_conv = mfh.converge()

where the user supplies an initial hamiltonian h0 and initial density matrix dm0.

@zerothi
Copy link
Collaborator

zerothi commented Feb 24, 2022

Yes, I think something like this would be better, convoluting namespaces is a dangerous path, I think.

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

No branches or pull requests

2 participants