-
Notifications
You must be signed in to change notification settings - Fork 21
Addign editor to fix atomic positions in x,y,z #1397
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: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1397 +/- ##
==========================================
- Coverage 72.40% 71.29% -1.12%
==========================================
Files 109 109
Lines 7317 7468 +151
==========================================
+ Hits 5298 5324 +26
- Misses 2019 2144 +125
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# Try to get fixed_atoms from AiiDA StructureData attributes | ||
fixed_atoms = None | ||
try: | ||
fixed_atoms = self.structure.base.attributes.all['fixed_atoms'] |
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.
Could you do something like fixed_atoms = self.structure.base.attributes.all.get('fixed_atoms', None) ?
chemical_symbols = ase_atoms.get_chemical_symbols() | ||
tags = ase_atoms.get_tags() | ||
|
||
def fmt_free(mask): |
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 think here, you can put a condition if mask is not an instance , mask = (0,0,0) and then you do return f"({' '.join('✓' if val else 'x' for val in mask)})"
src/aiidalab_qe/common/widgets.py
Outdated
self.input_selection = deepcopy(self.selection) | ||
|
||
|
||
class AddingFixedAtomsEditor(ipw.VBox): |
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 think you could name this editor, as ConstraintEditor , so in the future new constraints as implemented
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.
Done @AndresOrtegaGuerrero, thanks for the comments.
I have in mind how to modify the Editor to allow for other constraints, such as "distance", "torsion",..., but before we can do so, we would need such features implemented in the aiida plugin. Anyone willing to take the challenge @mbercx @superstar54 @edan-bainglass @AndresOrtegaGuerrero ?
We will also have to think about some warnings, since in QE, with symmetries, constraints can break the optimization.
connected to aiidalab/aiidalab-widgets-base#706


Constraints on x,y,z for each atom are stored in the ASE array 'fixed_atoms' .
The additional column is not present in case no constraints were specified
I need help (I would prefer in a separate PR), e.g. @edan-bainglass @superstar54, to add some warning logics:
If I load a structure that has constraints and I try to do phonons-related properties, I should get a warning. The warning can disappear only if 1)I remove the constraints, and 2) I do an additional geo-opt.
We should think about how to make this editor more general, including other types of constraints