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

Create mesh visualization #184

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
2ce7bef
add docstring
samwaseda May 17, 2024
df7611e
add it in __init__
samwaseda May 17, 2024
805c75b
small fixes
samwaseda May 17, 2024
393d2ea
Merge branch 'create_mesh' into create_mesh_visualization
samwaseda May 17, 2024
63fb47a
update type hinting
samwaseda May 17, 2024
979e739
Format black
pyiron-runner May 17, 2024
950dc17
Merge branch 'create_mesh' into create_mesh_visualization
samwaseda May 18, 2024
f619b6e
update the mesh definition
samwaseda May 18, 2024
28c0839
update error message
samwaseda May 18, 2024
e7590d9
Merge branch 'create_mesh' into create_mesh_visualization
samwaseda May 18, 2024
9e3a3e6
merge stuff
samwaseda May 19, 2024
d056bae
add type hinting
samwaseda May 19, 2024
9f5c0de
Merge branch 'create_mesh_visualization' of github.com:pyiron/structu…
samwaseda May 19, 2024
d5834af
add cell
samwaseda May 19, 2024
6efc237
Merge branch 'create_mesh' into create_mesh_visualization
samwaseda May 19, 2024
d555fa0
add box drawing functionality
samwaseda May 19, 2024
f1b9e4d
Merge branch 'create_mesh' into create_mesh_visualization
samwaseda May 20, 2024
0d6d577
change surface count
samwaseda May 21, 2024
c36b25f
Update structuretoolkit/visualize.py
samwaseda May 21, 2024
c334c05
Update structuretoolkit/visualize.py
samwaseda May 21, 2024
c27ab21
Format black
pyiron-runner May 22, 2024
aef5a64
Merge remote-tracking branch 'refs/remotes/origin/main' into create_m…
jan-janssen Jul 18, 2024
ffb67bf
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 18, 2024
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 structuretoolkit/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
)

# Visualize
from structuretoolkit.visualize import plot3d
from structuretoolkit.visualize import plot3d, plot_isosurface

# Analyse - for backwards compatibility
from structuretoolkit.analyse import (
Expand Down
79 changes: 78 additions & 1 deletion structuretoolkit/visualize.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
# Copyright (c) Max-Planck-Institut für Eisenforschung GmbH - Computational Materials Design (CM) Department
# Distributed under the terms of "New BSD License", see the LICENSE file.

from __future__ import annotations
import warnings

from ase.atoms import Atoms
import numpy as np
from typing import Optional
from typing import Optional, Union
from scipy.interpolate import interp1d
from structuretoolkit.common.helper import get_cell

from structuretoolkit.common.helper import get_cell

Expand Down Expand Up @@ -807,3 +809,78 @@ def _get_flattened_orientation(
flattened_orientation[:3, :3] = _get_orientation(view_plane)

return (distance_from_camera * flattened_orientation).ravel().tolist()


def plot_isosurface(
Copy link

Choose a reason for hiding this comment

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

Add missing import for Union.

The Union type hint is used but not imported. Import it from the typing module.

+from typing import Union
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def plot_isosurface(
+from typing import Union
def plot_isosurface(

mesh,
value,
cell: Optional[Union[Atoms, list, np.ndarray, float]] = None,
structure_plot: Optional["plotly.graph_objs._figure.Figure"] = None,
Copy link

Choose a reason for hiding this comment

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

Add missing import for plotly.

The plotly module is referenced in type hints but not imported. Import it to avoid undefined name errors.

+import plotly.graph_objects as go
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
structure_plot: Optional["plotly.graph_objs._figure.Figure"] = None,
import plotly.graph_objects as go
structure_plot: Optional["plotly.graph_objs._figure.Figure"] = None,
Tools
Ruff

818-818: Undefined name plotly

(F821)

isomin: Optional[float] = None,
isomax: Optional[float] = None,
surface_fill: Optional[float] = None,
opacity: Optional[float] = None,
surface_count: Optional[int] = 5,
samwaseda marked this conversation as resolved.
Show resolved Hide resolved
colorbar_nticks: Optional[int] = None,
caps: Optional[dict] = dict(x_show=False, y_show=False, z_show=False),
Copy link

Choose a reason for hiding this comment

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

Avoid mutable default arguments.

Using mutable default arguments can lead to unexpected behavior. Replace with None and initialize within the function.

-    caps: Optional[dict] = dict(x_show=False, y_show=False, z_show=False),
+    caps: Optional[dict] = None,

Initialize within the function:

if caps is None:
    caps = dict(x_show=False, y_show=False, z_show=False)
Tools
Ruff

825-825: Do not use mutable data structures for argument defaults

Replace with None; initialize within function

(B006)

colorscale: Optional[str] = None,
height: Optional[float] = 600,
samwaseda marked this conversation as resolved.
Show resolved Hide resolved
camera: Optional[str] = "orthographic",
**kwargs,
):
"""
Make a mesh plot

Args:
mesh (numpy.ndarray): Mesh grid. Must have a shape of (3, nx, ny, nz).
It can be generated from structuretoolkit.create_mesh
value: (numpy.ndarray): Value to plot. Must have a shape of (nx, ny, nz)
cell (Atoms|ndarray|list|float|tuple): Cell, ignored if
`structure_plot` is given
structure_plot (plotly.graph_objs._figure.Figure): Plot of the
Copy link
Member

Choose a reason for hiding this comment

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

I guess the type hint should show import directly from the public part of the API (https://plotly.com/python-api-reference/generated/plotly.graph_objects.Figure.html) and not ..._figure.Figure

structure to overlay. You should basically always use
structuretoolkit.plot3d(structure, mode="plotly")
isomin(float): Min color value
isomax(float): Max color value
surface_fill(float): Polygonal filling of the surface to choose between
0 and 1
opacity(float): Opacity
surface_count(int): Number of isosurfaces, 5 by default, which means
Copy link
Member

Choose a reason for hiding this comment

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

I believe you...but why? I would have thought it was 2 just like it was until the last commit (at time of writing). This is so weird it might warrant some sort of "I know, I know, but seriously, it's because X" in the docstring.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah I thought about it a little bit XD The problem is that somehow plotly sets the two layers for the highest value and the lowest value. It makes only sense if there are multiple points with the max/min value, but that's rarely the case. So even with 5, in reality you would see only 3 in the middle. I also thought about changing isomax and isomin, but that felt a bit like too much manipulation.

Copy link
Member

Choose a reason for hiding this comment

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

That's very fair. Maybe something like , which actually means only min and max because of details of how plotly handles the levels to explain the weirdness just a little?

only min and max
colorbar_nticks(int): Colorbar ticks correspond to isosurface values
caps(dict): Whether to set cap on sides or not. Default is False. You
can set: caps=dict(x_show=True, y_show=True, z_show=True)
colorscale(str): Colorscale ("turbo", "gnbu" etc.)
height(float): Height of the figure. 600px by default
camera(str): Camera perspective to choose from "orthographic" and
"perspective". Default is "orthographic"
"""
try:
import plotly.graph_objects as go
except ModuleNotFoundError:
raise ModuleNotFoundError("plotly not installed")
Copy link

Choose a reason for hiding this comment

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

Improve exception handling.

Use raise ... from None to distinguish exceptions from errors in exception handling.

-        raise ModuleNotFoundError("plotly not installed")
+        raise ModuleNotFoundError("plotly not installed") from None
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
raise ModuleNotFoundError("plotly not installed")
raise ModuleNotFoundError("plotly not installed") from None
Tools
Ruff

861-861: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

x_mesh = np.reshape(mesh, (3, -1))
data = go.Isosurface(
x=x_mesh[0],
y=x_mesh[1],
z=x_mesh[2],
value=np.array(value).flatten(),
isomin=isomin,
isomax=isomax,
surface_fill=surface_fill,
opacity=opacity,
surface_count=surface_count,
colorbar_nticks=colorbar_nticks,
caps=caps,
colorscale=colorscale,
**kwargs,
)
fig = go.Figure(data=data)
if structure_plot is not None:
fig = go.Figure(data=fig.data + structure_plot.data)
elif cell is not None:
fig = _draw_box_plotly(fig, cell)
fig.update_scenes(aspectmode="data")
fig.layout.scene.camera.projection.type = camera
fig.update_layout(autosize=True, height=height)
return fig
Loading