Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accounting for FHI-aims species' defaults #3877

Merged
merged 6 commits into from
Jun 20, 2024

Conversation

ansobolev
Copy link
Contributor

@ansobolev ansobolev commented Jun 12, 2024

Summary

This PR adds new SpeciesDefaults object to account for files with the default definitions of species-related keywords (integration grids, basis sets...) in FHI-aims, named species' defaults. This is done to abstract away the work with species' defaults from the AimsControlIn object, to bring the API more in line with VASP, and to enable running heterogeneous calculations that need different precision levels for different atoms.

Major changes:

  • An AimsSpeciesFile class representing the defaults for one species is introduced
  • A SpeciesDefaults class representing a list of species' defaults is introduced; the AimsControlIn class now uses SpeciesDefault
  • An AIMS_SPECIES_DIR configuration variable is introduced which should point to the root of FHIaims/species_defaults directory
  • Breaking change: the possibility to use default species' defaults directory is removed. Every input sets need explicit definition of the species' defaults directory relative to FHIaims/species_defaults or FHIaims/species_defaults/defaults_2020.

Todos

None

Checklist

  • Google format doc strings added. Check with ruff.
  • Type annotations included. Check with mypy.
  • Tests added for new features/fixes.
  • If applicable, new classes/functions/modules have duecredit @due.dcite decorators to reference relevant papers by DOI (example)

@tpurcell90
Copy link
Contributor

Would it be possible to reintroduce the default species_dir behavior? For some people using only one set of defaults is relatively okay

@tpurcell90
Copy link
Contributor

otherwise this looks good from an fhi-aims perspective. This will break atomate2 tests but we can fix that once it is merged

@ansobolev
Copy link
Contributor Author

Would it be possible to reintroduce the default species_dir behavior? For some people using only one set of defaults is relatively okay

Yeah, it could be possible (just remove one check and add species_dir to paths_to_try, I guess), but based on my discussions over here people also want to freely change the defaults, and they don't like writing out the full path every time.

@tpurcell90
Copy link
Contributor

Okay that makes sense

@ansobolev
Copy link
Contributor Author

Hi, is there anything that should be done here?

@JaGeo
Copy link
Member

JaGeo commented Jun 20, 2024

@ansobolev Thanks for contributing in any case! I alao hope one of the maintainers will soon have some time! 😃

@shyuep shyuep merged commit 4a385f9 into materialsproject:master Jun 20, 2024
33 checks passed
@shyuep
Copy link
Member

shyuep commented Jun 20, 2024

Thanks for the contribution. I have merged it!

@ansobolev ansobolev deleted the aims_fixes branch July 8, 2024 13:18
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.

4 participants