Skip to content
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

Implement viewer representations #373

Merged
merged 96 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from 87 commits
Commits
Show all changes
96 commits
Select commit Hold shift + click to select a range
e3ce568
draft
cpignedoli Oct 14, 2022
6768950
work in progress
cpignedoli Oct 15, 2022
445a3aa
first working version, to-do: add/remove atoms in edit structure
cpignedoli Oct 15, 2022
06a6f12
comments
cpignedoli Oct 15, 2022
9cea518
toward general formalism
cpignedoli Oct 16, 2022
1ea7067
starting modifiying addition and rmeova of atoms
cpignedoli Oct 16, 2022
5a12592
intermediate mess
cpignedoli Oct 17, 2022
f652396
used same concept of fragments for representations
cpignedoli Oct 17, 2022
469ef53
reordering representation class
cpignedoli Oct 17, 2022
9749526
first draft working
cpignedoli Oct 17, 2022
4670934
redesigned, debug prints present
cpignedoli Oct 18, 2022
83cbc81
refinements
cpignedoli Oct 18, 2022
c8006b6
use dtratlet input_structure
cpignedoli Oct 19, 2022
3d642d1
version for first evaluation
cpignedoli Oct 19, 2022
090b94a
problem of syncronization of arrays in structure traitlet
cpignedoli Oct 21, 2022
8467ce6
updated validate structure
cpignedoli Oct 21, 2022
745e6cb
Fixed bug of representrations not passed
cpignedoli Oct 21, 2022
e0f5318
default -1 for arrays['representations'] for atoms not included in a rep
cpignedoli Oct 21, 2022
a04e32c
added show of representations
cpignedoli Oct 21, 2022
755a6a4
modification
cpignedoli Oct 21, 2022
3ae05c3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 21, 2022
2957cef
simplifying to single component
cpignedoli Oct 22, 2022
f3d6310
bug fix
cpignedoli Oct 22, 2022
861c45d
bug fix
cpignedoli Oct 22, 2022
1a1ce85
strange ngl problme
cpignedoli Oct 23, 2022
bf627bb
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 23, 2022
c73fc68
first working version
cpignedoli Oct 24, 2022
8e86ecb
exclude from adavnced selection atoms not shown
cpignedoli Oct 24, 2022
fec6d9a
cleaning
cpignedoli Oct 24, 2022
9c19ecb
cleaning, removed orient_z_up to be put in separate PR
cpignedoli Oct 24, 2022
4a51a95
Merge branch 'master' into viewer-representations
yakutovicha Jan 12, 2023
61575a3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 12, 2023
0e19bcc
some flake fixes
cpignedoli Jan 16, 2023
a32d8b5
falke fix
cpignedoli Jan 17, 2023
09aff81
Small fixes, more compact representations styling.
yakutovicha Jan 23, 2023
d3a7cb5
Simplify the code.
yakutovicha Jan 24, 2023
2e26732
Continue with changes.
yakutovicha Jan 24, 2023
e7bef5e
Fix supercell appearance (old components were not removed).
yakutovicha Jan 25, 2023
fc4985d
Further simplify and improve the code.
yakutovicha Jan 25, 2023
ae7d8ad
Improve hightlight_atoms(), simplify the handling of representations.
yakutovicha Jan 26, 2023
fbc5a42
small, non-important changes.
yakutovicha Jan 26, 2023
79ac26f
Keep representations after editing the structure
yakutovicha Jan 30, 2023
227d09b
Apply suggestions from the code review.
yakutovicha Feb 1, 2023
bdda60d
Fix numpy array type problem.
yakutovicha Feb 1, 2023
85dcd83
Remove atoms selection from non-default representations when new stru…
yakutovicha Feb 1, 2023
3b0fdd1
Small change to a comment.
yakutovicha Feb 1, 2023
616a727
Merge branch 'master' into viewer-representations
yakutovicha Sep 25, 2023
76bc8e2
Fix bugs.
yakutovicha Sep 27, 2023
bd4e78c
Merge branch 'master' into viewer-representations
yakutovicha Sep 27, 2023
0ab456d
Fix bug for the editor.
yakutovicha Sep 28, 2023
d5c8320
Merge branch 'viewer-representations' of https://github.com/cpignedol…
yakutovicha Oct 6, 2023
bb3a536
Fix tests.
yakutovicha Oct 8, 2023
44e36a2
Merge branch 'master' into viewer-representations
yakutovicha Oct 8, 2023
5f190ec
Add tests for the viewer.
yakutovicha Oct 9, 2023
b0521eb
Merge branch 'viewer-representations' of https://github.com/cpignedol…
yakutovicha Oct 9, 2023
5ac5e79
Fix tests
yakutovicha Oct 9, 2023
2d2efee
Test 3 atoms selection.
yakutovicha Oct 9, 2023
263d966
More tests.
yakutovicha Oct 9, 2023
8f0559d
More tests.
yakutovicha Oct 9, 2023
8746939
More tests.
yakutovicha Oct 9, 2023
43a6776
Add tests for the representations.
yakutovicha Oct 9, 2023
ec9b553
Slightly improve datatypes viewers notebook.
yakutovicha Oct 9, 2023
38d19d3
More tests.
yakutovicha Oct 10, 2023
f033582
More tests.
yakutovicha Oct 10, 2023
c08712e
More tests.
yakutovicha Oct 10, 2023
37639fb
More tests.
yakutovicha Oct 10, 2023
1c1bc67
Review.
yakutovicha Oct 10, 2023
81ae8a9
Remove unnecessary warning check.
yakutovicha Oct 10, 2023
8bb2229
Merge branch 'master' into viewer-representations
yakutovicha Oct 12, 2023
13f9254
review
yakutovicha Oct 20, 2023
42b1d64
Improve selection color.
yakutovicha Oct 20, 2023
c38925b
Merge branch 'master' into viewer-representations
yakutovicha Oct 20, 2023
2be1f30
Representation arrays: convert them to bool type.
yakutovicha Oct 23, 2023
374128a
Fix pytests
yakutovicha Oct 23, 2023
5c82e8a
Fix tests.
yakutovicha Oct 23, 2023
9592cb3
Remove list_to_nglview function as it is used only in one place.
yakutovicha Oct 24, 2023
3b4843b
NglViewerRepresentation - improve class docstring, add comment for th…
yakutovicha Oct 24, 2023
9424bb7
fix
yakutovicha Oct 24, 2023
114959a
update sync_myself_to_array_from_atoms_object
yakutovicha Oct 24, 2023
781684f
Add type annotations.
yakutovicha Oct 24, 2023
58f6a37
Better name for the nglview parameters dictionary.
yakutovicha Oct 24, 2023
e75402f
natoms - make it a class property.
yakutovicha Oct 24, 2023
133167f
Put representation name into a constant
yakutovicha Oct 24, 2023
b6c84e1
_StructureDataBaseViewer work on docstrings.
yakutovicha Oct 25, 2023
255564e
delete_representation: add type hint.
yakutovicha Oct 25, 2023
694cf47
Slightly improve _observe_structure method.
yakutovicha Oct 25, 2023
523e207
try to fix docs build.
yakutovicha Oct 25, 2023
f533787
Representation arrays: move back to int to properly handle new atoms.
yakutovicha Oct 30, 2023
23f9a2f
Remove return statements from the 'add_myself_to_atoms_object' method.
yakutovicha Oct 31, 2023
66ccfbd
Replace "_aiidalab_viewer_representation_" with REPRESENTATION_PREFIX…
yakutovicha Oct 31, 2023
6be5d98
Fix issue with representation's name constant.
yakutovicha Nov 2, 2023
cb6e6bb
Split tests for better code maintainability.
yakutovicha Nov 2, 2023
f6ef4d1
review.
yakutovicha Nov 2, 2023
5abbf5d
Rename master_class with viewer_class
yakutovicha Nov 2, 2023
ed68daa
Merge branch 'master' into viewer-representations
yakutovicha Nov 6, 2023
0543067
Remove aiida_datatypes_viewers.ipynb
yakutovicha Nov 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion aiidalab_widgets_base/computational_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def refresh(self, _=None):
with self.hold_trait_notifications():
self.code_select_dropdown.options = self._get_codes()
if not self.code_select_dropdown.options:
self.output.value = f"No codes found for default calcjob plugin '{self.default_calc_job_plugin}'."
self.output.value = f"No codes found for default calcjob plugin {self.default_calc_job_plugin!r}."
unkcpz marked this conversation as resolved.
Show resolved Hide resolved
self.code_select_dropdown.disabled = True
else:
self.code_select_dropdown.disabled = False
Expand Down
6 changes: 3 additions & 3 deletions aiidalab_widgets_base/elns.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def connect_to_eln(eln_instance=None, **kwargs):
except (FileNotFoundError, json.JSONDecodeError, KeyError):
return (
None,
f"Can't open '{ELN_CONFIG}' (ELN configuration file). Instance: {eln_instance}",
f"Can't open {ELN_CONFIG!r} (ELN configuration file). Instance: {eln_instance}",
)

# If no ELN instance was specified, trying the default one.
Expand All @@ -35,7 +35,7 @@ def connect_to_eln(eln_instance=None, **kwargs):
eln_config = config[eln_instance]
eln_type = eln_config.pop("eln_type", None)
else: # The selected instance is not present in the config.
return None, f"Didn't find configuration for the '{eln_instance}' instance."
return None, f"Didn't find configuration for the {eln_instance!r} instance."

# If the ELN type cannot be identified - aborting.
if not eln_type:
Expand Down Expand Up @@ -73,7 +73,7 @@ def __init__(self, path_to_root="../", **kwargs):

if eln is None:
url = f"{path_to_root}aiidalab-widgets-base/notebooks/eln_configure.ipynb"
error_message.value = f"""Warning! The access to ELN is not configured. Please follow <a href="{url}" target="_blank">the link</a> to configure it.</br> More details: {msg}"""
error_message.value = f"""Warning! The access to ELN is not configured. Please follow <a href={url!r} target="_blank">the link</a> to configure it.</br> More details: {msg!r}"""
return

tl.dlink((eln, "node"), (self, "node"))
Expand Down
2 changes: 1 addition & 1 deletion aiidalab_widgets_base/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ def to_html_string(self):
return f"""
<table style="border-collapse:separate;border-spacing:15px;">
<tr>
<td style="width:200px"> <a href="{self.link}" target="_blank"> <img src="{self.logo}"> </a></td>
<td style="width:200px"> <a href={self.link!r} target="_blank"> <img src={self.logo!r}> </a></td>
<td style="width:800px"> <p style="font-size:16px;">{self.description} </p></td>
</tr>
</table>
Expand Down
39 changes: 26 additions & 13 deletions aiidalab_widgets_base/structures.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Module to provide functionality to import structures."""

import datetime
import functools
import io
Expand All @@ -14,7 +13,7 @@
from aiida import engine, orm, plugins

# Local imports
from .data import LigandSelectorWidget
from .data import FunctionalGroupSelectorWidget
danielhollas marked this conversation as resolved.
Show resolved Hide resolved
from .utils import StatusHTML, exceptions, get_ase_from_file, get_formula
from .viewers import StructureDataViewer

Expand All @@ -39,9 +38,7 @@ class StructureManagerWidget(ipw.VBox):
input_structure = tl.Union(
[tl.Instance(ase.Atoms), tl.Instance(orm.Data)], allow_none=True
)
structure = tl.Union(
[tl.Instance(ase.Atoms), tl.Instance(orm.Data)], allow_none=True
)
structure = tl.Instance(ase.Atoms, allow_none=True)
danielhollas marked this conversation as resolved.
Show resolved Hide resolved
structure_node = tl.Instance(orm.Data, allow_none=True, read_only=True)
node_class = tl.Unicode()

Expand Down Expand Up @@ -84,8 +81,8 @@ def __init__(
if viewer:
self.viewer = viewer
else:
self.viewer = StructureDataViewer(**kwargs)
tl.dlink((self, "structure_node"), (self.viewer, "structure"))
self.viewer = StructureDataViewer()
tl.dlink((self, "structure"), (self.viewer, "structure"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an important (and breaking) change, can you explain why this was needed? I think we've discussed this a bit when I was trying to merge the TrajectoryViewer but don't remember details. Are we sure this does not break anything?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was a bug to link structure_node. The structure trait is supposed to contain the current state of things, while structure_node is an unstored AiiDA object.

In the previous (buggy) version the AiiDA node would then be converted back to an ASE object losing loads of information.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, but I think there were some design decisions involved in that. I think some of the traitlets were meant to be immutable, while the conversion to ASE was for the editor? I am not sure. No action needed if everything works (has the BasicEditor been tested?). I will take a closer look when I have time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is a good idea to make the structure_node trait immutable because it is not meant to be modified externally. Its job is literally to be the StructureNode object corresponding to the structure trait (ASE).

Hmm, but I think there were some design decisions involved in that.

And, again, it was a mistake (probably mine) to link the structure_node trait instead of the structure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is a good idea to make the structure_node trait immutable because it is not meant to be modified externally.

structure_node trait is already read-only so I think we're good here. Thanks for the explanation. I'll leave this unresolved just to keep this discussion visible.


# Store button.
self.btn_store = ipw.Button(description="Store in AiiDA", disabled=True)
Expand Down Expand Up @@ -184,6 +181,10 @@ def _structure_editors(self, editors):
for i, editor in enumerate(editors):
editors_tab.set_title(i, editor.title)
tl.link((editor, "structure"), (self, "structure"))
if editor.has_trait("input_selection"):
tl.dlink(
(editor, "input_selection"), (self.viewer, "input_selection")
)
if editor.has_trait("selection"):
tl.link((editor, "selection"), (self.viewer, "selection"))
if editor.has_trait("camera_orientation"):
Expand Down Expand Up @@ -336,6 +337,7 @@ def _structure_changed(self, change=None):
This function enables/disables `btn_store` widget if structure is provided/set to None.
Also, the function sets `structure_node` trait to the selected node type.
"""

if not self.structure_set_by_undo:
self.history.append(change["new"])

Expand Down Expand Up @@ -1051,7 +1053,7 @@ def disable_element(_=None):
self.element.disabled = True

# Ligand selection.
self.ligand = LigandSelectorWidget()
self.ligand = FunctionalGroupSelectorWidget()
self.ligand.observe(disable_element, names="value")

# Add atom.
Expand Down Expand Up @@ -1262,6 +1264,8 @@ def translate_dr(self, _=None, atoms=None, selection=None):
self.action_vector * self.displacement.value
)

self.input_selection = None # Clear selection.

self.structure, self.input_selection = atoms, selection

@_register_structure
Expand All @@ -1271,7 +1275,7 @@ def translate_dxdydz(self, _=None, atoms=None, selection=None):

# The action.
atoms.positions[self.selection] += np.array(self.str2vec(self.dxyz.value))

self.input_selection = None # Clear selection.
yakutovicha marked this conversation as resolved.
Show resolved Hide resolved
self.structure, self.input_selection = atoms, selection

@_register_structure
Expand All @@ -1281,7 +1285,7 @@ def translate_to_xyz(self, _=None, atoms=None, selection=None):
# The action.
geo_center = np.average(self.structure[self.selection].get_positions(), axis=0)
atoms.positions[self.selection] += self.str2vec(self.dxyz.value) - geo_center

self.input_selection = None # Clear selection.
self.structure, self.input_selection = atoms, selection

@_register_structure
Expand All @@ -1295,6 +1299,7 @@ def rotate(self, _=None, atoms=None, selection=None):
center = self.str2vec(self.point.value)
rotated_subset.rotate(self.phi.value, v=vec, center=center, rotate_cell=False)
atoms.positions[self.selection] = rotated_subset.positions
self.input_selection = None # Clear selection.

self.structure, self.input_selection = atoms, selection

Expand Down Expand Up @@ -1329,6 +1334,8 @@ def mirror(self, _=None, norm=None, point=None, atoms=None, selection=None):
# Mirror atoms.
atoms.positions[selection] -= 2 * projections

self.input_selection = None # Clear selection.

self.structure, self.input_selection = atoms, selection

def mirror_3p(self, _=None):
Expand Down Expand Up @@ -1375,6 +1382,7 @@ def mod_element(self, _=None, atoms=None, selection=None):
initial_ligand = self.ligand.rotate(
align_to=self.action_vector, remove_anchor=True
)

for idx in self.selection:
position = self.structure.positions[idx].copy()
lgnd = initial_ligand.copy()
Expand All @@ -1390,22 +1398,23 @@ def mod_element(self, _=None, atoms=None, selection=None):
@_register_selection
def copy_sel(self, _=None, atoms=None, selection=None):
"""Copy selected atoms and shift by 1.0 A along X-axis."""

last_atom = atoms.get_global_number_of_atoms()

# The action
add_atoms = atoms[self.selection].copy()
add_atoms.translate([1.0, 0, 0])
atoms += add_atoms

new_selection = list(range(last_atom, last_atom + len(selection)))
self.structure, self.input_selection = atoms, new_selection
self.structure, self.input_selection = atoms, list(
range(last_atom, last_atom + len(selection))
)

@_register_structure
@_register_selection
def add(self, _=None, atoms=None, selection=None):
"""Add atoms."""
last_atom = atoms.get_global_number_of_atoms()

if self.ligand.value == 0:
initial_ligand = ase.Atoms([ase.Atom(self.element.value, [0, 0, 0])])
rad = SYMBOL_RADIUS[self.element.value]
Expand All @@ -1431,6 +1440,8 @@ def add(self, _=None, atoms=None, selection=None):

new_selection = list(range(last_atom, last_atom + len(selection) * len(lgnd)))

# The order of the traitlets below is important -
# we must be sure trait atoms is set before trait selection
self.structure, self.input_selection = atoms, new_selection

@_register_structure
Expand All @@ -1439,6 +1450,8 @@ def remove(self, _=None, atoms=None, selection=None):
"""Remove selected atoms."""
del [atoms[selection]]

# The order of the traitlets below is important -
# we must be sure trait atoms is set before trait selection
self.structure = atoms
self.input_selection = None
yakutovicha marked this conversation as resolved.
Show resolved Hide resolved
self.input_selection = []
2 changes: 1 addition & 1 deletion aiidalab_widgets_base/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def predefine_settings(obj, **kwargs):
if hasattr(obj, key):
setattr(obj, key, value)
else:
raise AttributeError(f"'{obj}' object has no attribute '{key}'")
raise AttributeError(f"{obj!r} object has no attribute {key!r}")


def get_ase_from_file(fname, file_format=None): # pylint: disable=redefined-builtin
Expand Down
2 changes: 1 addition & 1 deletion aiidalab_widgets_base/utils/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ class ListOrTuppleError(TypeError):

def __init__(self, value):
super().__init__(
f"The provided value '{value}' is not a list or a tupple, but a {type(value)}."
f"The provided value {value!r} is not a list or a tupple, but a {type(value)}."
)
Loading