Feature DFT+U #577
Feature DFT+U #577
Conversation
|
Before i continue to the final steps of the PR (like the independent tab) I was wondering if you @superstar54 and @unkcpz could give me your input on this PR design :D |
|
I have decided that the tab to display the occupation should be better in a second PR, otherwise this one will be too big |
|
@AndresOrtegaGuerrero thanks! I'll give it a look.
I agree, and since it is for the next release. I think even better steps are using your bandsplot widget to replace the current one first (link to bandsplot PR #581). |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #577 +/- ##
==========================================
- Coverage 80.73% 75.64% -5.09%
==========================================
Files 49 60 +11
Lines 3415 4644 +1229
==========================================
+ Hits 2757 3513 +756
- Misses 658 1131 +473
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Very cool! I didn't get to too much detail yet. But the first thing I notice is this create widget from an elements pattern which appeared in the magnetization setting and pseudopotential setting. I think the third time it appears to push me to provide a commend interface for the design. I'll try to work on that first. |
|
@superstar54 @unkcpz Thank you , I will work on the changes, I did notice something |
|
For this PR to be evaluated we need the following PRs to be merged |
tests/configuration/test_advanced.py
Outdated
| # Create a StructureData for AdvancedSettings (LiCoO2) | ||
|
|
||
| a, b, c, d = 1.4060463552647, 0.81178124180108, 4.6012019181836, 1.6235624832021 | ||
| cell = [[a, -b, c], [0.0, d, c], [-a, -b, c]] | ||
| sites = [ | ||
| ["Co", "Co", (0, 0, 0)], | ||
| ["O", "O", (0, 0, 3.6020728736387)], | ||
| ["O", "O", (0, 0, 10.201532881212)], | ||
| ["Li", "Li", (0, 0, 6.9018028772754)], | ||
| ] | ||
| structure = orm.StructureData(cell=cell) | ||
|
|
||
| for site in sites: | ||
| structure.append_atom(position=site[2], symbols=site[0], name=site[1]) | ||
|
|
There was a problem hiding this comment.
Since this structure may be useful to test other properties, could you move this to conftest.py and create a fixture for it.
There was a problem hiding this comment.
thank you , i will add it in conftest.py
| for x in self.input_structure.kinds | ||
| ] | ||
| result = [ | ||
| f"{kind} - {manifold}" |
There was a problem hiding this comment.
There is possible risk. If a kind has a name "Co - up", then the label will be "Co - up - 3d", this will raise an error in the Workchain when it tries to get the kind and manifold back.
There was a problem hiding this comment.
but usually we dont allow names as Co-up, we allow tags like Co1, or Co2 ,
There was a problem hiding this comment.
Yes, in QEApp, users can only add tags like Co1 and Co2. But this can be improved in the future. We may support tags like Mn3+, Mn4+, Co-up, Co-down, etc. Also, users may import data from others that already have tags like Co-up and load it from the AiiDA database to QEApp.
There was a problem hiding this comment.
But i guess this should be control @ the structure step then , dont you think ?
There was a problem hiding this comment.
Yes, the tags are controlled in the structure step. However, the problem here is the {kind} - {manifold} could result in something like Co-up - 3d (e.g. if user import data from database, and the kind name is Co-up already), thus it will raise an error in the WorkChain, where you have one line of code
kind, orbital = key.split(" - ")
I suggest you split by the last - to solve this problem, or you can use a dict with the kind and orbital saved explicitly.
There was a problem hiding this comment.
@superstar54 so for something like this , kind, orbital = key.rsplit("-", 1)?
There was a problem hiding this comment.
Done, let me know
|
@superstar54 Before merging, we need to request a new release of aiida-quantumespresso plugin, and bump the version here. So it doesn't fail when considering 1D, 2D systems for bands and so on. |
| hubbard_parameters = hubbard_dict["hubbard_u"] | ||
| hubbard_structure = HubbardStructureData.from_structure(structure) | ||
| for key, value in hubbard_parameters.items(): | ||
| kind, orbital = key.rsplit("-", 1) |
There was a problem hiding this comment.
There are spaces when creating the label.
| kind, orbital = key.rsplit("-", 1) | |
| kind, orbital = key.rsplit(" - ", 1) |
This PR add the feature to the Qe App to run simulations with DFT+U on-site
The feature include a HubbardWidget, where the hubbard manifold is only one orbital,
for example for Co will the the 3d , of for Si it will be the 3p
(This can modified so the widget display the valence orbital shell , for example Co 3d and 4s) (Or if you prefer the the previous occupied shell like 3p for Co, we can discuss about this :D ) ,
For a second PR:
I would like to have an independent tab for displaying DFT+U results (of interest) like the occupation matrix, and the occupation per atom and spin. I was wondering if this should be implemented as a plugin (Like in electronic_structure plugin) but i think the results should be available independently of the workchain run (bands, pdos, or an ext plugin)