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

Allow Structure.interpolate to extrapolate #3467

Merged
merged 9 commits into from
Nov 29, 2023

Conversation

kyledmiller
Copy link
Contributor

@kyledmiller kyledmiller commented Nov 10, 2023

Summary

Major changes:

  • added optional parameter to allow Structure.interpolate() to extrapolate beyond the end_structure. The parameter is the fractional amount of extrapolation beyond $q=1$ if $q$ is the normalized interpolation mode amplitude. Thus, extrapolation=0.5 would produce structures spanning $q \in [0,1.5]$. This is helpful for capturing the outer piece of potential energy wells, e.g., when fitting a Landau model to an 2nd order structural phase transition.

Todos

Create tests

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)

Tip: Install pre-commit hooks to auto-check types and linting before every commit:

pip install -U pre-commit
pre-commit install

@janosh janosh added enhancement A new feature or improvement to an existing one needs testing PRs that are not ready to merge due to lacking tests core Pymatgen core labels Nov 26, 2023
@@ -2138,6 +2138,7 @@ def interpolate(
interpolate_lattices: bool = False,
pbc: bool = True,
autosort_tol: float = 0,
extrapolation: float = 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test for this new keyword?

@kyledmiller
Copy link
Contributor Author

kyledmiller commented Nov 27, 2023 via email

…ion vector connecting structure to end_structure. This allows for reverse inter/extrapolation as well as partial inter/extrapolation. With this argument, 0 implies no distortion, 1 implies full distortion to end_structure (default), 0.5 implies distortion to a point halfway between structure and end_structure, and -1 implies full distortion in the opposite direction to end_structure.
@kyledmiller kyledmiller marked this pull request as ready for review November 29, 2023 18:24
@kyledmiller
Copy link
Contributor Author

Tests written

Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests look great! Just 1 line length issue left before we can merge.

@kyledmiller
Copy link
Contributor Author

Looks good. I fixed that and also added some additions about cell setting & site order consistency to the docstring. If you don't think it belongs or should be in a separate PR, let me know.

@janosh janosh enabled auto-merge (squash) November 29, 2023 19:07
@janosh janosh merged commit b35bda3 into materialsproject:master Nov 29, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Pymatgen core enhancement A new feature or improvement to an existing one needs testing PRs that are not ready to merge due to lacking tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants