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

Quasiharmonic Approximation: Use the same size of supercell for both the energy volume curve and the phonon calculation #119

Merged
merged 3 commits into from
Dec 11, 2023

Conversation

jan-janssen
Copy link
Member

No description provided.

…the energy volume curve and the phonon calculation.
@jan-janssen jan-janssen added the format_black Launch the pyiron/actions black formatting routine label Dec 11, 2023
Copy link
Member

@liamhuber liamhuber left a comment

Choose a reason for hiding this comment

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

Super. One suggestion for adding a comment to explain the intent of the repetition to future maintainers.

I'm inferring that using the same interaction_range in both get_supercell_matrix and PhonopyWorkflow like this is guaranteeing the behaviour; ideally I'd like to see a test that this is the case, but it's straightforward enough that I can tolerate skipping that.

@samwaseda
Copy link
Member

Wait do I see it correctly that it's now hard coded? From my point of view it totally makes sense to use a small cell for the energy volume curve, so I would make it only optional

@jan-janssen
Copy link
Member Author

Wait do I see it correctly that it's now hard coded? From my point of view it totally makes sense to use a small cell for the energy volume curve, so I would make it only optional

This should be fine, as the energy is calculated at the same time as the forces, so it basically was a bug before.

@jan-janssen jan-janssen merged commit 109e783 into main Dec 11, 2023
17 checks passed
@jan-janssen jan-janssen deleted the qh_same_size branch December 11, 2023 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format_black Launch the pyiron/actions black formatting routine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants