search

nomad-coe / nomad-simulations

A NOMAD plugin containing base sections for simulations.
https://nomad-coe.github.io/nomad-simulations/
Apache License 2.0
5 stars 1 forks source link

Issues with DFT XC extraction #115

Open JosePizarro3 opened 2 months ago

JosePizarro3 commented 2 months ago

Matyas Novak from the SPRKKR is trying to write a parser and found some errors. This is the message he sent me:

1) I ecounter an error ValueError: max() arg is an empty sequence raised from nomad_simulations/schema_packages/model_method.py line 352, where the following line highest_rung_abbrev = max(abbrevs, key=lambda x: rung_order[x]) failed, if abbrevs is empty - which could be. There is check for None, but not for an empty list.

2) Moreover, there is probably an another bug: during normalize in nomad_simulations/schema_packages/model_method.py(202)normalize() the libxc_name is from same reason altered as follows: self.libxc_name = libxc_name_weight + self.libxc_name

However, in the code mentined above in the method resolve_jacobs_ladder line 340 of the nomad_simulations/schema_packages/model_method.py does not take the "weight" preffix into account, so the name is not parsed and the result is the same exception, as in 1, since the list of known names is empty

@ndaelman-hu my guess is that the normalization of the DFT and XCFunctional class is broken. Would you mind taking a look and taking this chance to work on #13 and polishing the DFT schema? It shouldn't be complicated.

ndaelman-hu commented 2 months ago

I pushed a branch fixing issue 1. Regarding issue 2, I need some more context: this piece of code was implemented in function of GW, but I'm not sure what purpose it serves. All I can gather is some code dev 6 years ago decided to generate a long-format search string. Typically, functional names are used directly. There's definitely some benefit in searching all functionals that contain a certain percentage of one, but this format isn't very conducive to it. My first thought is to simply remove this.

JosePizarro3 commented 2 months ago

All boils down to:

  1. Check the DFT schema and,
  2. have some testing

I'll merge your fix.

ndaelman-hu commented 1 month ago

Regarding issue 2, I need some more context: this piece of code was implemented in function of GW, but I'm not sure what purpose it serves.

@JosePizarro3 I was actually directing this question to you, but I should've been a bit more explicit.

From what I gather, you introduced functional_long_name_from_method in the legacy code, which then got ported over. Could you pls give me some more explanation on its original purpose? That would help with deciding on the next action. It's of course possible that you moved it over from somewhere else, but the trail ends there.

JosePizarro3 commented 1 month ago

I am not sure what you meant with "...was implemented in function of GW.". If it is a piece of code old and useless, you are welcome to delete it.

It seems something with the libxc name and the weighting, but my expertise finishes there. I just moved everything to keep consistent about the old normalization.

ndaelman-hu commented 1 month ago

I am not sure what you meant with "...was implemented in function of GW.". If it is a piece of code old and useless, you are welcome to delete it.

I meant that git traces it back to a commit named "Hotfix GW workflow method section".

It seems something with the libxc name and the weighting, but my expertise finishes there. I just moved everything to keep consistent about the old normalization.

Okay, thank you for clarifying