-
Notifications
You must be signed in to change notification settings - Fork 24
Fix inconsistent strain convention: implement proper Voigt notation with engineering shear strains and consistent variable naming #80
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
base: master
Are you sure you want to change the base?
Conversation
…trains Modified get_strain() to return engineering shear strains (2× tensor components) and updated all symmetry functions (regular, tetragonal, orthorombic, trigonal, hexagonal, monoclinic, triclinic) to accept Voigt notation without multiplying by 2. This resolves the inconsistency where manual elastic constant calculations would produce C44 values twice as large as expected. Co-authored-by: jochym <5993422+jochym@users.noreply.github.com>
Added a note to explain that the mathematical formulation in the documentation uses tensor components while the code returns engineering strains in proper Voigt notation. Both formulations are equivalent. Co-authored-by: jochym <5993422+jochym@users.noreply.github.com>
Corrected the monoclinic and triclinic symmetry functions to properly handle coupling terms without introducing an extra factor of 1/2. The coupling terms should remain unchanged when converting from tensor to Voigt notation. Co-authored-by: jochym <5993422+jochym@users.noreply.github.com>
Fixed RST syntax in documentation (removed :python: directive) and added explanatory comment in trigonal function about why some factors of 2 remain due to crystallographic symmetry relationships. Co-authored-by: jochym <5993422+jochym@users.noreply.github.com>
|
@copilot This is still not consistent. If we move to Ei notation (Voight) we should not use tensor streins in the naming of variables inside the functions. Can you make it completely consistent? |
…ry functions Changed variable names from tensor notation (uxx, uyy, uzz, uyz, uxz, uxy) to Voigt notation (e1, e2, e3, e4, e5, e6) in all symmetry functions: regular(), tetragonal(), orthorombic(), trigonal(), hexagonal(), monoclinic(), triclinic(). This makes the code completely consistent with Voigt notation convention. Co-authored-by: jochym <5993422+jochym@users.noreply.github.com>
Done! Renamed all variables from tensor notation (uxx, uyy, uzz, uyz, uxz, uxy) to Voigt notation (e1, e2, e3, e4, e5, e6) throughout all symmetry functions. The code is now completely consistent with Voigt/engineering strain notation. (commit 339a1be) |
Fixed a bug in the tetragonal function where e4 and e5 were swapped in rows 4 and 5. The correct mapping is: row 4 (σ4/σyz) uses e4 (εyz), row 5 (σ5/σxz) uses e5 (εxz). This bug existed in the original code and was accidentally preserved during variable renaming. Co-authored-by: jochym <5993422+jochym@users.noreply.github.com>
get_strain()returned tensor shear components (ε₂₃, ε₁₃, ε₁₂) but claimed to return Voigt notation. Proper Voigt notation uses engineering shear strains (2ε₂₃, 2ε₁₃, 2ε₁₂). This caused manual one-axis elastic constant calculations to produce C₄₄ values that were 2× too large. Additionally, variable names used tensor notation (uxx, uyy, uyz, etc.) throughout the symmetry functions, creating confusion about the actual convention used.Changes
Strain computation
get_strain()now returns[ε₁₁, ε₂₂, ε₃₃, 2ε₂₃, 2ε₁₃, 2ε₁₂]instead of[ε₁₁, ε₂₂, ε₃₃, ε₂₃, ε₁₃, ε₁₂]Symmetry functions (accept Voigt notation input)
regular(),tetragonal(),orthorombic(),hexagonal()regular()row 4 changed from[0, 0, 2*uyz]to[0, 0, e4]monoclinic()andtriclinic()(no factor change needed)trigonal()(distinct from Voigt convention)uxx, uyy, uzz, uyz, uxz, uxy) to Voigt notation (e1, e2, e3, e4, e5, e6) throughout all symmetry functions for complete consistencyBug fixes
tetragonal()where e4 and e5 were swapped in rows 4 and 5Documentation
Example
This is a breaking change for code that expected tensor components from
get_strain(), but aligns with standard elasticity conventions and the function's documented behavior. The variable naming now clearly indicates Voigt notation is used throughout.Original prompt
This section details on the original issue you should resolve
<issue_title>Inconsistent treatment of strain throughout the code</issue_title>
<issue_description>First of all, thank you so much @jochym for sharing your Elastic code with the community.
Here I would like to report my concern about the convention of strain in the code.
To my best knowledge, in the common definition of the Voigt notation for strain, the strain values$\epsilon_1, \cdots, \epsilon_6$ are related to the tensor components as follows,
$$
\begin{pmatrix}
\epsilon_{11} &
\epsilon_{12} &
\epsilon_{13} \
\epsilon_{21} &
\epsilon_{22} &
\epsilon_{23} \
\epsilon_{31} &
\epsilon_{32} &
\epsilon_{33}
\end{pmatrix} =
\begin{pmatrix}
\epsilon_{1} &
\epsilon_{6} / 2 &
\epsilon_{5} / 2 \
\epsilon_{6} / 2 &
\epsilon_{2} &
\epsilon_{4} / 2 \
\epsilon_{5} / 2 &
\epsilon_{4} / 2 &
\epsilon_{3}
\end{pmatrix},
$$
i.e.,$\epsilon_4 = 2 \epsilon_{23}$ , where we have the coefficient 2.
This is different from the convention for the stress (without the coefficient 2).
Formally, the shear components in the Voigt notation are equal to “engineering” shear strains.
At least in the context of elastic properties, the Voigt notation is not just replacements of indices.
And, the common stress-strain relation like (for cubic):
$$
\begin{align}
\begin{pmatrix}
\sigma_{1} \
\sigma_{2} \
\sigma_{3} \
\sigma_{4} \
\sigma_{5} \
\sigma_{6} \
\end{pmatrix}
&=
\begin{pmatrix}
B_{11} & B_{12} & B_{12} & 0 & 0 & 0 \
B_{12} & B_{11} & B_{12} & 0 & 0 & 0 \
B_{12} & B_{12} & B_{11} & 0 & 0 & 0 \
0 & 0 & 0 & B_{44} & 0 & 0 \
0 & 0 & 0 & 0 & B_{44} & 0 \
0 & 0 & 0 & 0 & 0 & B_{44} \
\end{pmatrix}
\begin{pmatrix}
\epsilon_{1} \
\epsilon_{2} \
\epsilon_{3} \
\epsilon_{4} \
\epsilon_{5} \
\epsilon_{6} \
\end{pmatrix}
\end{align}
$$
is written using the above common Voigt notation.
I confirmed these points with several textbooks (Nye and Grimvall).
My concern is that the strain treatment is not well consistent through the code. Specifically,
get_strain: It returns stress, not in the common Voigt notation, but actually just as an array of tensor components.regular,hexagonal, …: They must take the tensor-component strains rather than those in the Voigt notation.get_cart_deformed_cell(and thusget_elementary_deformations): here, the shear strains are engineering strains and thus the strain must be given in the Voigt notation.Particularly for the non-Voigt convention of
get_strainmay cause confusion for readers in realistic cases:get_strain.To demonstrate the issue, I made a test using ASE EMT potential Cu_EMT.zip:
jochym0I usedget_elementary_deformationsandget_elastic_tensorand obtained C11 = 172 GPa, C12 = 116 GPa, C44 = 90 GPajochym1I used the one-axis approach usingget_cart_deformed_celland linear fitting and obtained C11 = 173 GPa, C12 = 115 GPa, C44 = 180 GPa, where C44 is twice larger obtained. This is because the strain is given not in the common Voigt notation.To solve the issue, I wish to suggest:
get_strainmethod, the returned values should not be called “in the Voigt notation”.get_strainmethod can be modified to return strain in the common Voigt notation. This also requires corresponding changes in other methods likeregular,hexagonal, etc.It would be very appreciated if you kindly think of these points above. Thank you so much for your help in advance.
</issue_description>
Comments on the Issue (you are @copilot in this section)
@jochym @yuzie00...✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.