dipc-cc / hubbard

Python tools for mean-field Hubbard models
https://dipc-cc.github.io/hubbard/
GNU Lesser General Public License v3.0
21 stars 8 forks source link

HubbardHamiltonian as child class of sisl.Hamiltonian? #119

Open tfrederiksen opened 2 years ago

tfrederiksen commented 2 years ago

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 commented 2 years ago

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, https://github.com/zerothi/sisl/issues/97. That would be ideal, IMHO.

tfrederiksen commented 2 years ago

Thanks, I had forgotten about https://github.com/zerothi/sisl/issues/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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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