update lobstertask schema: add bandoverlaps,grosspop and sitepotentials fields

[naik-aakash]

Update Lobstertaskschema

• Added sitepotentials, bandoverlaps, grosspopulations field to schema
• Update associated tests

[naik-aakash]

Hi @JaGeo , it would be great if you can also have a look at it when feasible

[codecov]

Codecov Report

Merging #404 (3ad31f6) into main (ce0a6f7) will decrease coverage by 0.08%.
The diff coverage is 90.90%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #404      +/-   ##
==========================================
- Coverage   65.03%   64.95%   -0.08%     
==========================================
  Files          74       74              
  Lines        7192     7214      +22     
  Branches      947      952       +5     
==========================================
+ Hits         4677     4686       +9     
- Misses       2216     2228      +12     
- Partials      299      300       +1     

Impacted Files	Coverage Δ
src/atomate2/lobster/schemas.py	92.74% <90.90%> (-0.17%)	⬇️

... and 1 file with indirect coverage changes

[JaGeo]

This looks great to me.

One question:
Do you have a test case without band overlaps? Does the schema need to know in advance if the band overlaps are present?

[naik-aakash]

This looks great to me.

One question: Do you have a test case without band overlaps? Does the schema need to know in advance if the band overlaps are present?

Hi, Schema checks if bandoverlaps are present, if not then field is set to None.

And the tests in flow already checks the case where the band overlaps file is not present 😃.
So in that case I check if field value is set to None.
https://github.com/naik-aakash/atomate2/blob/cc01a1ec8a224c471c83f3dee1086b1f62e83fbb/tests/vasp/lobster/flows/test_lobster.py#L99
https://github.com/naik-aakash/atomate2/blob/cc01a1ec8a224c471c83f3dee1086b1f62e83fbb/tests/vasp/lobster/flows/test_lobster.py#L173

[JaGeo]

Maybe, you can do a test run on 50 compounds and check if you can recreate all relevant and also the new objects from thr schema 😃.

Then, it's fine from my side.

[naik-aakash]

Maybe, you can do a test run on 50 compounds and check if you can recreate all relevant and also the new objects from thr schema 😃.

Then, it's fine from my side.

Yes, on it and will update here and let you know when it is done so it can be merged 😄

[JaGeo]

Great :).

[naik-aakash]

Hi @utf , I think this PR is ready to be merged. Can you please review it and if you have any further suggestions, I would be happy to address it

[JaGeo]

Hi @utf, @naik-aakash and I would kindly ask you to have a look at this pull request. We are currently working on a resubmission and a merged pull request would help a lot with regard to repoducibility.
Thank you very much in advance.

[JaGeo]

Not sure how the maintainer list works here at the moment. @janosh, In case you are allowed and have some time, we would love a review because of the issues mentioned above.

[utf]

Sorry for the delay. This all looks good to me!

[janosh]

@JaGeo I only do reviews and merge minor uncontentious-looking PRs (like docs, linting, tests).

[JaGeo]

Thanks, @utf !