Skip to content

Remove auto_nbands handling + fix bravais inconsistency handling#409

Merged
Andrew-S-Rosen merged 11 commits intomaterialsproject:masterfrom
esoteric-ephemera:master
Dec 18, 2025
Merged

Remove auto_nbands handling + fix bravais inconsistency handling#409
Andrew-S-Rosen merged 11 commits intomaterialsproject:masterfrom
esoteric-ephemera:master

Conversation

@esoteric-ephemera
Copy link
Copy Markdown
Contributor

Major changes:

  • Change auto_nbands to only throw a warning and not attempt to change the VASP job, as per original intention. VASP already updates NBANDS to accommodate parallelization settings so the handler here is doing more harm (esp. in bandstructure sweeps) than good
  • Update the bravais handling to reflect advice from VASP devs and the docstr that's there

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 20, 2025

Codecov Report

❌ Patch coverage is 26.66667% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.74%. Comparing base (312d4cf) to head (5e3749d).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/custodian/vasp/handlers.py 0.00% 22 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #409      +/-   ##
==========================================
+ Coverage   53.66%   53.74%   +0.07%     
==========================================
  Files          39       39              
  Lines        3557     3541      -16     
==========================================
- Hits         1909     1903       -6     
+ Misses       1648     1638      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Andrew-S-Rosen
Copy link
Copy Markdown
Member

Andrew-S-Rosen commented Dec 14, 2025

@esoteric-ephemera: There are some merge conflicts now, unfortunately. Would you be willing to resolve them? Once you do, I'd be happy to consider this for merging.

@Andrew-S-Rosen
Copy link
Copy Markdown
Member

Andrew-S-Rosen commented Dec 14, 2025

Note: This PR would also close #368 by making it obsolete.

@esoteric-ephemera
Copy link
Copy Markdown
Contributor Author

@Andrew-S-Rosen there's something a bit off with the one lobster test, will figure it out today

@Andrew-S-Rosen
Copy link
Copy Markdown
Member

@esoteric-ephemera --- this looks good to me. Ready for merge?

@esoteric-ephemera
Copy link
Copy Markdown
Contributor Author

Yep should be good! It'd be good to eventually try to patch the tests to be more robust. I was getting huge numbers of files written out running everything locally with pytest-xdist

@Andrew-S-Rosen
Copy link
Copy Markdown
Member

try to patch the tests to be more robust.

Agreed. But for another day...

@Andrew-S-Rosen Andrew-S-Rosen merged commit dc49153 into materialsproject:master Dec 18, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants