Fix WenAlloys.compute_gamma_radii to match documented formula (#952)#967
Open
vasa-develop wants to merge 1 commit into
Open
Fix WenAlloys.compute_gamma_radii to match documented formula (#952)#967vasa-develop wants to merge 1 commit into
vasa-develop wants to merge 1 commit into
Conversation
…gmaterials#952) compute_gamma_radii computed the solid-angle numerator/denominator as (r_bar*r + r**2) instead of the documented ((r + r_bar)**2 - r_bar**2), so the implementation disagreed with both its own docstring and the Wang et al. (Scripta Mater. 94, 2015) definition gamma = omega_S/omega_L. Correct the formula, update the affected pinned values in test_WenAlloys, and add a focused regression test for compute_gamma_radii.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #952.
WenAlloys.compute_gamma_radiiimplements the atomic-packing parameter γ = ω_S / ω_L (solid angles of the smallest and largest atoms) from Wang et al., Scripta Materialia 94 (2015) 28–31 (doi:10.1016/j.scriptamat.2014.09.010), whereThe implementation expanded the numerator inside the square root incorrectly:
(r + r_bar)^2 - r_bar^2expands to2*r_bar*r + r^2, but the code usedr_bar*r + r^2— i.e. it dropped oner_bar*rterm. This made the code disagree with both its own docstring and the source paper. (Reported by @KwamiAldo in #952.)Fix
The docstring already documented the correct (paper) formula, so this only changes the implementation to match it.
Impact
This changes the value of the
"Radii gamma"feature. For example, with mean/min/max Miracle radii of 140/120/160 pm, γ changes from1.189to1.362— which straddles the commonly used γ ≈ 1.175 solid-solution vs. intermetallic threshold, so the correction can affect downstream phase-stability screening. The pinned"Radii gamma"values intest_WenAlloysare updated accordingly.Test plan
test_wen_alloys_gamma_radii, a focused regression test that checkscompute_gamma_radiiagainst the closed-form ω_S/ω_L expression (and a hard-coded analytic value). It fails onmain(old formula gives1.1888…) and passes here."Radii gamma"values intest_WenAlloysto the corrected outputs.pytest tests/featurizers/composition/test_alloy.pypasses (5 passed).black --check(line-length 120) clean on the changed files; no newflake8issues introduced.Note
This PR intentionally fixes only
compute_gamma_radii. The related #962 also flags the configuration-entropy sign and the mixing-enthalpyabs(); I've kept those out of scope here so this change stays minimal and easy to review. (For what it's worth, the "Yang omega is affected" claim in #962 does not appear to hold —YangSolidSolution.compute_omegawraps the result inabs(), so the entropy sign cancels there — but happy to follow up separately on the entropy/enthalpy features if you'd like.)