aiidateam / aiida-quantumespresso-hp

MIT License
3 stars 0 forks source link

`SelfConsistentHubbardWorkChain`: Relabeling of `HubbardStructure` if relaxations are skipped #63

Closed t-reents closed 5 months ago

t-reents commented 7 months ago

Fixes #61.

@bastonero I moved the relabeling to a separate method, as discussed. I decided to introduce a new method in SelfConsistentHubbardWorkChain, instead of a new utility function in a separate module, to directly access the context of the WorkChain. The relabeling logic is the same as before and the method takes also care of generating an appropriate message to be passed to self.report.

In this new version, the HubbardStructure would also be relabeled if meta_convergence == False. If you would like to stick to the current logic, in the sense, that it's not updated, I could simply add an if-statement to the method. This is up to you.

The name of the method is of course debatable as well. I wasn't a 100% happy with the current name, as it also checks whether an update is necessary and might not relabel the structure. However, something like should_relabel... would indicate that a bool is returned. Moreover, update_hubbard... seemed a bit too general.

Tests have been adjusted as well and I added another one, to check if the correct reporting messages are provided.

t-reents commented 7 months ago

Thanks for your comments @bastonero! I agree that this simplified logic makes more sense and that the messages are probably not necessary. I only introduced this logic to keep the previous reports etc., and to not mess things up too much. But as you don't think they are necessary as well, I'm more than happy to remove them.

bastonero commented 5 months ago

Hi @t-reents , did you use this feature lately? Does everything work? If yes, I guess we could merge this then.

t-reents commented 5 months ago

Hi @bastonero, I used it after I implemented it and everything seems to work, so I think it can be merged

bastonero commented 5 months ago

Thanks a lot @t-reents !