-
Notifications
You must be signed in to change notification settings - Fork 78
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
Periodicity: StructureData #907
Changes from 4 commits
da5c4cd
d330787
fa70e86
bf1fd9c
a3dab3b
52b69fc
13fe734
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -143,7 +143,21 @@ def get_builder_from_protocol( | |
base.pw.parameters['CELL']['cell_dofree'] = 'shape' | ||
|
||
if relax_type in (RelaxType.CELL, RelaxType.POSITIONS_CELL): | ||
base.pw.parameters['CELL']['cell_dofree'] = 'all' | ||
x_dim, y_dim, z_dim = structure.pbc | ||
if x_dim and not (y_dim or z_dim): | ||
base.pw.parameters['CELL']['cell_dofree'] = 'x' | ||
elif y_dim and not (x_dim or z_dim): | ||
base.pw.parameters['CELL']['cell_dofree'] = 'y' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you are more of an expert, but I'm wondering: assume you have a structure with CELL_PARAMETERS angstrom
13.866974650 0.0000000000 0.0000000000
1.9334873250 3.3488982827 0.0000000000
1.9334873250 1.1162994276 13.157371580 I’m wondering if I set cell_dofree to 'y', will only the second component of the second lattice parameter (i.e. 3.3488982827) be allowed to change? If only the second component is allowed to change, also the direction of the lattice vector will change, which could deform the chain structure unless all atoms are on the second lattice vector, no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this scenario , and according to the manual There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mbercx Perhaps if there are so many options for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, it seems you typically would make sure the Xth lattice vector only has an X component. E.g.
Would be a reasonable cell with
I understand this will probably be quicker for your use case, and it possible avoids creating bugs in the main There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mbercx Ok, I will do the changes you suggested, I am pro removing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AndresOrtegaGuerrero if it's ok for you, I was already doing the changes and adding a test, but got distracted by aiidateam/aiida-core#5967 when I actually tried to import a 2D structure. 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mbercx no problem! , thank you There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, seems I can't push to the and |
||
elif z_dim and not (x_dim or y_dim): | ||
base.pw.parameters['CELL']['cell_dofree'] = 'z' | ||
elif x_dim and y_dim and not z_dim: | ||
base.pw.parameters['CELL']['cell_dofree'] = '2Dxy' | ||
elif x_dim and y_dim and z_dim: | ||
base.pw.parameters['CELL']['cell_dofree'] = 'all' | ||
else: | ||
raise ValueError( | ||
'Invalid combination of periodic boundary conditions.' | ||
) # what other combinations are possible? | ||
|
||
builder = cls.get_builder() | ||
builder.base = base | ||
|
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 see, so this correction can only be used when the first two lattice vectors are entirely in the x-y plane. So running with
(False, True, True)
should not be allowed for this reason? Should the third lattice vector be perpendicular to this plane?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.
@mbercx Thank you for the question. So I included this option given the suggestion from @demiranda-gabriel , Usually If i were to do a slab simulation i would omit this part and just be sure that my structure has enough vacuum in the perpendicular direction of the slab. According to the manual this option is only for xy-plane. Though I am pro removing this line.
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.
Hmm, since the
get_builder_from_protocol
method is meant to provide a set of sensible defaults, it might be reasonable to keep it since the user can always override it. One sentence in the QE documentation still worries me:@demiranda-gabriel So QE would not consider any z-components of the forces? Wouldn't we want to include these when e.g. optimizing the surface layers?
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.
Nevermind, it clearly does. The initial forces and stresses:
are reduced to
Input/output cell: