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
added FCC dislocations to dislocation module #151
Conversation
This was broken for the dissociated dislocations: the symbol was not assigned correctly.
For the disslciated dislocations I switched to copying the properties from one of the provided partials rather than hardcoding it again. Also added the internal testing of the parameters of the two partials and resulting burgers vector.
matscipy/dislocation.py
Outdated
@@ -629,8 +635,11 @@ def make_screw_cyl_kink(alat, C11, C12, C44, | |||
b = np.sqrt(3.0) * alat / 2.0 | |||
cent_x = np.sqrt(6.0) * alat / 3.0 | |||
|
|||
disloc_ini, disloc_fin, bulk_ini = make_barrier_configurations((alat, C11, C12, C44), | |||
cylinder_r=cylinder_r, **kwargs) | |||
disloc_ini, \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better style to avoid need for \
line continuation by enclosing values within parentheses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, I should try that. I did it just because of the flake8 linter complains about the lengths of the line. I saw in the guidelines that we do not care much about it. So I see what looks more readable. I agree that \
are ugly
@@ -55,7 +57,8 @@ | |||
class TestDislocation(matscipytest.MatSciPyTestCase): | |||
"""Class to store test for dislocation.py module.""" | |||
|
|||
@unittest.skipIf("atomman" not in sys.modules, 'requires Stroh solution from atomman to run') | |||
@unittest.skipIf("atomman" not in sys.modules, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recall a plan to replace atomman with our own Stroh solution. Is that still something you'd like to do? Doesnt't have to be be as part of this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I would like to do that because atomman has too many dependencies. Same goes for the Vitek plots and this is work in progress.
Looks good, thanks @pgrigorev. I'll be happy to merge anyway, but I left a couple of optional comments for you to consider. |
Thank you very much for such a quick response! I will happily rebase and take into account other comments. |
Thanks! |
The main enhancement is the addition of 1/2<110>{111} edge and screw dislocations in FCC. In FCC these dislocations dissociate into two 1/6<112>{111} Shockley partials as for example: 1/2[1-10] = 1/6<2-1-1> + ISF + 1/6<1-21>.
The implementation includes:
FCCScrewShockleyPartial
- Shockley screw partialFCCScrew110Dislocation
- dissociated <110> screw dislocationFCCEdgeShockleyPartial
- Shockley edge partialFCCEdge110Dislocation
- dissociated <110> edge dislocationI also added relevant test and fixed few bugs.
CubicCrystalDissociatedDislocation
was significantly reworked. Now it checks that provided partials have same properties and the sum of the burgers vectors results in the desired value. After that the properties of the resulting dislocation are taken from one of the partials. It makes the code more readable and robust.