nomad-coe / electronic-parsers

Apache License 2.0
18 stars 7 forks source link

Implement pdos cp2k #151

Closed JosePizarro3 closed 8 months ago

JosePizarro3 commented 11 months ago

Implementing projected DOS for CP2K.

I had to define a convolution function which could be generalized to be moved to utils.py.

JosePizarro3 commented 11 months ago

Hi @ladinesa ,

These are the changes for parsing the DOS in CP2K. You can review it after we merge your fix #149

Just a few remarks:

A final remark: the convolution is rather sensible to the width and delta_energy. I suggest we also store this metainfo quantities in case someone wants to work out their DOS themselves. Later we can think of improving this convolution with more automatic numbers.

I will work now on testing.

JosePizarro3 commented 11 months ago

I had to add one by one for the parse_calculations cases so that parse_dos is properly called to the corresponding calculation section. Check it out please.

JosePizarro3 commented 11 months ago

@ladinesa please check the new version regarding the PDOS files and their iteration step. Also, check the testing.

Let me know what you think, thanks!

JosePizarro3 commented 10 months ago

Ok, I will push soon the changes into nomad. Let's leave this PR here until then (spin_channels is not defined in the metainfo, so it would give problems).

JosePizarro3 commented 10 months ago

@ladinesa , I am not sure if you are back or not. But no worries, just wanted to let you know I was working on the parsers to include the new Dos schema. This is related with: https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/merge_requests/1449

I know this might not have been optimal, but I wanted to check the parsers to seek potential bugs (and I found a couple of things).

If you are off for a long period, let me know and I ask Nathan or Lauri to review this 🙂

JosePizarro3 commented 10 months ago

@ladinesa , I am not sure if you are back or not. But no worries, just wanted to let you know I was working on the parsers to include the new Dos schema. This is related with: https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/merge_requests/1449

I know this might not have been optimal, but I wanted to check the parsers to seek potential bugs (and I found a couple of things).

If you are off for a long period, let me know and I ask Nathan or Lauri to review this 🙂

Btw, I have a testing folder specifically for DOS, in case you want to check this out.

ladinesa commented 10 months ago

@ladinesa , I am not sure if you are back or not. But no worries, just wanted to let you know I was working on the parsers to include the new Dos schema. This is related with: https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/merge_requests/1449

I know this might not have been optimal, but I wanted to check the parsers to seek potential bugs (and I found a couple of things).

If you are off for a long period, let me know and I ask Nathan or Lauri to review this 🙂

I am on leave til 15.9, slowly getting back to work this week. I can also have a look but I do not mind if others can have a look also.

ladinesa commented 10 months ago

Seems ok to me. Maybe change the pr description to Implement new dos schema since you also applied it to all? electronic parsers

ondracka commented 9 months ago

BTW I think LOBSTER parser will also need updates for the new metainfo.

However, maybe it should be moved here to electronic from the workflow parsers. I believe the reason why it is there is that it is not a DFT code, but now with Wannier90 living in electronic parsers, LOBSTER is pretty much the same thing, i.e., it takes a DFT wavefunctions, does a projection and than calculates few properties.

ndaelman-hu commented 9 months ago

BTW I think LOBSTER parser will also need updates for the new metainfo.

However, maybe it should be moved here to electronic from the workflow parsers. I believe the reason why it is there is that it is not a DFT code, but now with Wannier90 living in electronic parsers, LOBSTER is pretty much the same thing, i.e., it takes a DFT wavefunctions, does a projection and than calculates few properties.

Hey, thanks for pointing that out. Would you mind opening an issue and assigning it to me?

JosePizarro3 commented 9 months ago

BTW I think LOBSTER parser will also need updates for the new metainfo.

However, maybe it should be moved here to electronic from the workflow parsers. I believe the reason why it is there is that it is not a DFT code, but now with Wannier90 living in electronic parsers, LOBSTER is pretty much the same thing, i.e., it takes a DFT wavefunctions, does a projection and than calculates few properties.

Hi Pavel,

Thanks a lot! Indeed, I misregarded the LOBSTER parser, and I will adapt it as well.

About the project structure, I think this has to be reworked a bit. Furthermore, there is more than Wannier90 and LOBSTER currently in the parsers which might fall also into workflows category. Let's think about that :)

ondracka commented 9 months ago

BTW I think LOBSTER parser will also need updates for the new metainfo. However, maybe it should be moved here to electronic from the workflow parsers. I believe the reason why it is there is that it is not a DFT code, but now with Wannier90 living in electronic parsers, LOBSTER is pretty much the same thing, i.e., it takes a DFT wavefunctions, does a projection and than calculates few properties.

Hey, thanks for pointing that out. Would you mind opening an issue and assigning it to me?

You mean an issue about whether the LOBSTER should be in workflow parsers (or potentially Wannier90 in electronic)? Or about the missing metainfo?

ndaelman-hu commented 9 months ago

BTW I think LOBSTER parser will also need updates for the new metainfo. However, maybe it should be moved here to electronic from the workflow parsers. I believe the reason why it is there is that it is not a DFT code, but now with Wannier90 living in electronic parsers, LOBSTER is pretty much the same thing, i.e., it takes a DFT wavefunctions, does a projection and than calculates few properties.

Hey, thanks for pointing that out. Would you mind opening an issue and assigning it to me?

You mean an issue about whether the LOBSTER should be in workflow parsers (or potentially Wannier90 in electronic)? Or about the missing metainfo?

The metainfo, pls

JosePizarro3 commented 9 months ago

Wait wait... these changes regarding the new refactor of the DOS will be implemented by me, @ndaelman-hu.

And for moving the LOBSTER to electronic-parsers, I pretty much agree, so feel free to go for it, just let me know if you want to do it after or before I merge this (probably before is faster).

ondracka commented 9 months ago

Opened https://github.com/nomad-coe/workflow-parsers/issues/27 in the meantime, but maybe you guys just figure it out and than close it if needed.

JosePizarro3 commented 8 months ago

And for moving the LOBSTER to electronic-parsers, I pretty much agree, so feel free to go for it, just let me know if you want to do it after or before I merge this (probably before is faster).

I am coming back to this to clarify that we might be re-organizing the projects structure following: https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/issues/1744. But it will be done after these new PDOS changes are implemented.