Add raw parser of Lowdin charges from projwfc output file

[chrisjsewell]

This is intended to be merged after #424, then used to add an output node to ProjwfcCalculation containing this data.

[sphuber]

Same comment as previous PR. Merging 600k lines is suboptimal :) is it possible to reduce this one as well?

[chrisjsewell]

Yeh geez that's a big file, perhaps these files shouldn't be fully stored in the retrieved folder for projwfc? Maybe trim them to only contain the first/last x lines.
Anyway fixed in ded77ac

[sphuber]

Thanks @chrisjsewell . Would it make sense to actually add this to the ProjwfcParser and add the lowdin charges as an output node? Maybe making it optional through the settings input node?

[chrisjsewell]

Add the lowdin charges as an output node.

I think maybe you didn't read my first comment lol. Do you want me to add that in this PR?

[chrisjsewell]

@sphuber In aac5790 I have added the lodwin output node (and cleaned up the rest of the parser code a bit). I haven't done anything too fancy with it, just left it as an orm.Dict, but I do add the uuid for the parent structure, to make it easy to match up the charges with structure sites.

I haven't made it optional, since it doesn't take up too much memory. On this point though, I did a little analysis of the memory resources taken up by the ProjwfcCalculation for one of my calculations:

from pathlib import Path

projwfc = orm.load_node(50845)
for linkt in projwfc.get_outgoing().all():
    path = linkt.node._repository._repo_folder.abspath
    size = sum(f.stat().st_size 
               for f in Path(path).glob('**/*') if f.is_file())
    print(f"{linkt.link_label+':':20s} {size/1e6:5.2f} Mb")

Dos:                  0.03 Mb
bands_down:           0.17 Mb
projections_down:    16.65 Mb
bands_up:             0.17 Mb
projections_up:      16.65 Mb
output_parameters:    0.00 Mb
retrieved:           37.13 Mb
remote_folder:        0.00 Mb

This isn't doing the size of my repository any favors!

As I mentioned above, perhaps aiida.out should be moved to the retrieve_temporary_list?
The downside of this is obviously that you couldn't inspect or re-parse the output at a later date.

Two middle grounds would be to either:

• save aiida.out, but remove the orbital data section, that makes the file so big
• zipping the file reduces its size from 31 Mb -> 5 Mb. This is maybe a more general suggestion that I wanted to make in aiida-core, to add a ZipFile data node. For example, I have implemented this in aiida_crystal17.data.GaussianCube

[sphuber]

Thanks for the analysis, very useful. I would be in favor of moving the output file to retrieved temporary. The parsing should be relatively complete and if one really would need to reparse, one could rerun the calculation, even though that is still not super convenient. But it would save almost 50% of disk space, which is a lot. Regarding the zipping: I just opened a new AiiDA enhancement proposal to improve the repository, which should come with support for packing and compressing repository contents.

[chrisjsewell]

@sphuber e395edd moves aiida.out to retrieved temporary. I also added a default test for ProjwfcCalculation, and added a fixture for parsing nodes with temporary folders, which can be removed when aiidateam/aiida-core#3061 is closed.

[sphuber]

@chrisjsewell I opened a PR to allow passing retrieved_temporary_folder to parse through the parse_from_node call. Let me know if that works so I can merge it and this PR can be adapted

[chrisjsewell]

@shuber, I guess you meant aiidateam/aiida-core#3446, but yes that's sorted ta. I have no more changes to make, but I guess that means this PR will need to wait until aiida-core>1.0.0b6 is released?

[sphuber]

but I guess that means this PR will need to wait until aiida-core>1.0.0b6 is released?

Yes, but since we are releasing aiida-core==1.0.0 next week, I don't think that is a problem. Hope you don't mind either. We will merge this then as soon as v1.0 is out.

[chrisjsewell]

Yes, but since we are releasing aiida-core==1.0.0 next week, I don't think that is a problem. Hope you don't mind either. We will merge this then as soon as v1.0 is out.

Finally lol, yeh that's no problem

[sphuber]

Hi @chrisjsewell I had to put aiida-quantumespresso on the back burner for a while, but I would like to merge this before the end of next week so I can put it in the 3.0.0 release. If you can rebase on current develop and remove the workaround for the parsing with retrieved_temporary_folder, that would be great.

Also, I recently homogenized all exit codes across all calculation jobs in PR #479 please apply the same conventions to your new exit codes for ProjwfcCalculation.

Thanks a lot

[giovannipizzi]

Hi @chrisjsewell , this has been open for quite a while now.
Do you think you'll have time to rebase and clean up?
Also, you might want to check with @ramirezfranciscof both for help in reviewing, and also for him as he needs to compute some charges (not really Löwdin ones I think, but might be useful to check)

[mbercx]

@chrisjsewell if it's alright by you, I'll add these changes in with #749 and make you a co-author there.

[chrisjsewell]

and make you a co-author there.

I want my attribution!
yeh that's fine though 👍

[mbercx]

Then I will close this PR. I've made a note to add the changes to #749 so I don't forget. 😁