adapting BandsWorkChain for HubbardStructureData

[AndresOrtegaGuerrero]

When using HubbardStructureData with PwBandsStructure the workchain doesnt use the hubbard information. Since it calls seekpath , the information is lost.

Additionally the HubbardStructureData has a bug for StructureData with periodicity 1d and 2d.

This PR addresses these two issues , #999

[bastonero]

Thanks @AndresOrtegaGuerrero. I am wondering if then all the methods are working as expected. Could you please provide some tests and/or examples, with some structure? I think there should be already some fixtures for 2D. It is important that the functions work, and in the intended behaviour. Here, it is not clear if your fix is going to work out.

[AndresOrtegaGuerrero]

Hi @bastonero , The 2D and 1D is already working at the PwBaseWorkChain (like the sampling of k-points) we did these with @mbercx. Regarding the PwBandsWorkChain, this branch manage to fix the bug. Another test is the PdosWorkChain that works without no problems with HubbardStructureData. Is also tested in the AiiDAlab qe app (https://github.com/aiidalab/aiidalab-qe/pull/577)

[AndresOrtegaGuerrero]

I think you meant to add test for the hubbard structure, and for bands workchain 😅

[mbercx]

Thanks @AndresOrtegaGuerrero! One more general note here is that you are making two quite different changes in one PR:

1. Provide support for low-dimensional structures to HubbardStructureData.
2. Provide support for HubbardStructureData to the PwBandsWorkChain.

No need to open another PR, but I'd split these changes up into two commits that describe each. Happy to do this once the PR is happy to merge, or discuss how to do this over Zoom.

[AndresOrtegaGuerrero]

Thanks @AndresOrtegaGuerrero! One more general note here is that you are making two quite different changes in one PR:

1. Provide support for low-dimensional structures to HubbardStructureData.
2. Provide support for HubbardStructureData to the PwBandsWorkChain.

No need to open another PR, but I'd split these changes up into two commits that describe each. Happy to do this once the PR is happy to merge, or discuss how to do this over Zoom.

Thank you @mbercx sorry about the two changes, in my head they were quite small that since it was an aiidalab_qe issue i thought it was ok in one. It wont happen again :D

[bastonero]

The pbc should be fixed by this PR https://github.com/aiidateam/aiida-core/pull/6281#issue-2125449409

[AndresOrtegaGuerrero]

@bastonero , @mbercx proposed a solution here without the need of using generate_structure

[mbercx]

@AndresOrtegaGuerrero @bastonero I'll move the seekpath-related changes to a different PR so we can focus on the <3D structures here.

I've added some validation on the site indices for append_hubbard_parameter in f8d6933.

I've also extended the tests of the method to check more cases and verify that adapting it for <3D structures doesn't break anything. ^^

[mbercx]

@bastonero do you still want to have a look?

[bastonero]

@bastonero do you still want to have a look?

Let's have a quick one

[bastonero]

Lol, I had a huge review with all the comments that nobody was seeing. That's why nobody was replying to those comments

[mbercx]

Lol, I had a huge review with all the comments that nobody was seeing. That's why nobody was replying to those comments

Haha, I read "let's have a quick one", then saw the massive review and almost fell out of my chair. 🤣

[mbercx]

Now working on https://github.com/aiidateam/aiida-quantumespresso/pull/1007, will consider your comments there, but we already agreed that we'd stick to the onsite case for now.

[bastonero]

@mbercx it seems you have to self approve yourself :D

[mbercx]

@mbercx it seems you have to self approve yourself :D

Oh right, I requested changes after all 😅

[mbercx]

Haha, seems we also need @unkcpz's approval now 😂