Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
13 changes: 8 additions & 5 deletions pyphare/pyphare/pharesee/hierarchy/hierarchy.py
Original file line number Diff line number Diff line change
Expand Up @@ -583,11 +583,14 @@ def dist_plot(self, **kwargs):

if finest:
final = finest_part_data(self)
if axis[0] == "x":
xbins = amr_grid(self, time)
bins = (xbins, vbins)
else:
bins = (vbins, vbins)
if len(axis) == 2:
if axis[0] == "x":
xbins = amr_grid(self, time)
bins = (xbins, vbins)
else:
bins = (vbins, vbins)
elif len(axis) == 1:
bins = vbins
kwargs["bins"] = bins

else:
Expand Down
145 changes: 103 additions & 42 deletions pyphare/pyphare/pharesee/plotting.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

kwargs:
* axis : ("x", "Vx"), ("x", "Vy"), ("x", "Vz"), ("Vx", "Vy") (default) --
("Vx", "Vz"), ("Vy", "Vz")
("Vx", "Vz"), ("Vy", "Vz"), ("Vx"), (Vy"), ("Vz")
* bins : number of bins in each dimension, default is (50,50)
Comment on lines 20 to 22
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix malformed axis docstring entry.
Line 21 has (Vy"), which reads as a typo and can confuse users.

✏️ Suggested fix
-       ("Vx", "Vz"), ("Vy", "Vz"), ("Vx"), (Vy"), ("Vz")
+       ("Vx", "Vz"), ("Vy", "Vz"), ("Vx"), ("Vy"), ("Vz")
🤖 Prompt for AI Agents
In `@pyphare/pyphare/pharesee/plotting.py` around lines 20 - 22, The docstring for
the "axis" parameter in pyphare.pyphare.pharesee.plotting.py contains a
malformed entry `(Vy")`; update the axis list so all entries are properly quoted
and comma-separated (e.g., replace `(Vy")` with ("Vy") and ensure consistency
like ("Vx"), ("Vy"), ("Vz") and pairs ("Vx","Vy") etc.) in the plotting module's
docstring so the axis options are readable and valid.

* gaussian_filter_sigma : sigma of the gaussian filter, default is (0,0)
* median_filter_size : size of the median filter, default is (0,0)
Expand Down Expand Up @@ -53,50 +53,111 @@
axis = kwargs.get("axis", ("Vx", "Vy"))
vaxis = {"Vx": 0, "Vy": 1, "Vz": 2}

if axis[0] in vaxis:
x = particles.v[:, vaxis[axis[0]]]
elif axis[0] == "x":
x = particles.x
if axis[1] in vaxis:
y = particles.v[:, vaxis[axis[1]]]
if len(axis) == 2:
if axis[0] in vaxis:
x = particles.v[:, vaxis[axis[0]]]
elif axis[0] == "x":
x = particles.x
else:
raise ValueError("Only abscissa and velocity X-axis are supported yet")
Copy link
Member

Choose a reason for hiding this comment

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

do we consider the option (y,v) or (z,v) or (v,y) and (v,z) ?

if axis[1] in vaxis:
y = particles.v[:, vaxis[axis[1]]]
else:
raise ValueError("Only velocity Y-axis are supported yet")

bins = kwargs.get("bins", (50, 50))
h, xh, yh = np.histogram2d(
x, y, bins=bins, weights=particles.weights[:, 0]
)

bins = kwargs.get("bins", (50, 50))
h, xh, yh = np.histogram2d(
x, y, bins=kwargs.get("bins", bins), weights=particles.weights[:, 0]
)
if "gaussian_filter_sigma" in kwargs and "median_filter_size" not in kwargs:
from scipy.ndimage import gaussian_filter

sig = kwargs.get("gaussian_filter_sigma", (0, 0))
image = gaussian_filter(h.T, sigma=sig)
elif "median_filter_size" in kwargs and "gaussian_filter_sigma" not in kwargs:
from scipy.ndimage import median_filter

siz = kwargs.get("median_filter_size", (0, 0))
image = median_filter(h.T, size=siz)
elif "gaussian_filter_sigma" not in kwargs and "median_filter_size" not in kwargs:
image = h.T
else:
raise ValueError(
"gaussian and median filters can not be called at the same time"
)

plain = kwargs.get("plain", False)

if not plain:
cmap = kwargs.get("cmap", "jet")

cmax = kwargs.get("color_max", h.max())
cmin = kwargs.get("color_min", h.min())
cmin = max(cmin, 1e-4)

color_scale = kwargs.get("color_scale", "log")
if color_scale == "log":
norm = LogNorm(vmin=cmin, vmax=cmax)
elif color_scale == "linear":
norm = Normalize(cmin, cmax)
else:
raise ValueError("Only log and linear color_scale values are supported yet")

im = ax.pcolormesh(xh, yh, image, cmap=cmap, norm=norm)

fig.colorbar(im, ax=ax)
else:
color = kwargs.get("color", "k")
stride = kwargs.get("stride", 1)
markersize = kwargs.get("markersize", 0.5)
alpha = kwargs.get("alpha", 0.5)
im = ax.plot(x[::stride], y[::stride],

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable im is not used.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, like the one defined with the previous pcolormesh. But lets keep this handle around !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, but let's keep it,

color=color,
linewidth=0,
Copy link
Member

Choose a reason for hiding this comment

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

I think the more correct way to do this plot would be to use ax.scatter() rather than a plot with a 0 size linewidth (which rather should even be linestyle="none")

marker='.',
markersize=markersize,
alpha=alpha)
Comment on lines +111 to +120
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Validate stride to avoid invalid slicing.
Line 112 allows stride=0, which raises ValueError in slicing.

🧩 Proposed guard
         stride = kwargs.get("stride", 1)
+        if stride <= 0:
+            raise ValueError("stride must be a positive integer")
         markersize = kwargs.get("markersize", 0.5)
🤖 Prompt for AI Agents
In `@pyphare/pyphare/pharesee/plotting.py` around lines 111 - 120, The code reads
stride = kwargs.get("stride", 1) and then uses x[::stride] which fails for
stride=0; update the plotting function to validate/coerce stride before slicing:
ensure stride is an integer >= 1 (e.g. convert via int() and if <= 0 either set
stride = 1 or raise a ValueError with a clear message), and then use that
validated stride when calling ax.plot (the variables to look for are the stride
assignment and the ax.plot call where x[::stride], y[::stride] are used).


ax.set_ylabel(kwargs.get("ylabel", axis[1]))


elif len(axis) == 1:
Copy link
Member

Choose a reason for hiding this comment

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

I think axis should be listified (pyphare.core.phare_utilities.listify) to allow users who just want one axis to do axis="x" rather than having to type axis=("x",)

bins = kwargs.get("bins", (50))
cuts = kwargs.get("cuts", None)
ndim = particles.ndim

if cuts is not None:
from pyphare.core.box import Box
if ndim == 1:
box_new = Box(cuts[0][0], cuts[0][1])
new_particles = particles.select(box_new, box_type="pos")
Comment on lines +127 to +134
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Handle None entries in cuts as described by the API.
The code assumes every cuts[i] is a tuple; None will throw at cuts[0][0].

🧩 One‑dimensional fix (minimal)
         if cuts is not None:
             from pyphare.core.box import Box
             if ndim == 1:
-                box_new = Box(cuts[0][0], cuts[0][1])
-                new_particles = particles.select(box_new, box_type="pos")
+                if cuts[0] is None:
+                    new_particles = particles
+                else:
+                    box_new = Box(cuts[0][0], cuts[0][1])
+                    new_particles = particles.select(box_new, box_type="pos")
🤖 Prompt for AI Agents
In `@pyphare/pyphare/pharesee/plotting.py` around lines 127 - 134, The code
assumes cuts[0] is a tuple and fails if it is None; update the ndim==1 branch to
handle None entries by checking cuts[0] is not None before constructing Box and
calling particles.select: if cuts[0] is None leave new_particles as the original
particles (or skip selection), otherwise create box_new = Box(cuts[0][0],
cuts[0][1]) and set new_particles = particles.select(box_new, box_type="pos");
ensure this pattern (check for None) is applied wherever cuts[i] is used so
particles.select is only called with a valid Box.

elif ndim == 2:
box_new = Box((cuts[0][0], cuts[1][0]), (cuts[0][1], cuts[1][1])) # TODO need to be tested
new_particles = particles.select(box_new, box_type="pos")
else:
box_new = Box((cuts[0][0], cuts[1][0], cuts[2][0]), (cuts[0][1], cuts[1][1], cuts[2][1])) # TODO need to be tested
new_particles = particles.select(box_new, box_type="pos")
else:
new_particles = particles

if "gaussian_filter_sigma" in kwargs and "median_filter_size" not in kwargs:
from scipy.ndimage import gaussian_filter
drawstyle = kwargs.get("drawstyle", "steps-mid")

sig = kwargs.get("gaussian_filter_sigma", (0, 0))
image = gaussian_filter(h.T, sigma=sig)
elif "median_filter_size" in kwargs and "gaussian_filter_sigma" not in kwargs:
from scipy.ndimage import median_filter
if axis[0] in vaxis:
x = new_particles.v[:, vaxis[axis[0]]]
else:
raise ValueError("For 1-D dist_plot, the abscissa has to be a velocity axis")
Comment on lines +146 to +149
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

KDE path breaks for 1‑D axis.
When len(axis)==1, y is never defined, but Line 165 still calls sns.kdeplot(x=x, y=y, ...).

🧩 Proposed fix
     if kwargs.get("kde", False) is True:
         import seaborn as sns
-
-        sns.kdeplot(x=x, y=y, ax=ax, color="w")
+        if len(axis) == 1:
+            sns.kdeplot(x=x, ax=ax, color="w")
+        else:
+            sns.kdeplot(x=x, y=y, ax=ax, color="w")

Also applies to: 162-165

🧰 Tools
🪛 GitHub Check: CodeQL

[notice] 149-149: Unused local variable
Variable im is not used.

🪛 Ruff (0.14.13)

149-149: Avoid specifying long messages outside the exception class

(TRY003)

🤖 Prompt for AI Agents
In `@pyphare/pyphare/pharesee/plotting.py` around lines 146 - 149, The 1-D branch
fails because y is never defined but sns.kdeplot is later called with y=y; in
the block handling len(axis)==1 (when axis[0] in vaxis), set y=None (or call
sns.kdeplot without the y argument) and ensure the subsequent call to
sns.kdeplot uses only x when y is None; update the code paths around the
axis/vaxis check and the sns.kdeplot invocation (references: axis, vaxis,
new_particles.v, and sns.kdeplot) so that for 1-D plots you pass only x (or x
and y=None) and avoid referencing an undefined y (apply same fix to the other
occurrence around lines 162-165).


siz = kwargs.get("median_filter_size", (0, 0))
image = median_filter(h.T, size=siz)
elif "gaussian_filter_sigma" not in kwargs and "median_filter_size" not in kwargs:
image = h.T
else:
raise ValueError(
"gaussian and median filters can not be called at the same time"
bins = kwargs.get("bins", 50)
h, xh = np.histogram(
x, bins=bins, weights=new_particles.weights[:, 0]
)

cmap = kwargs.get("cmap", "jet")

cmax = kwargs.get("color_max", h.max())
cmin = kwargs.get("color_min", h.min())
cmin = max(cmin, 1e-4)
xh_ = 0.5*(xh[:-1]+xh[1:])
im = ax.plot(xh_, h, drawstyle=drawstyle)

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable im is not used.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

see above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the same,


color_scale = kwargs.get("color_scale", "log")
if color_scale == "log":
norm = LogNorm(vmin=cmin, vmax=cmax)
elif color_scale == "linear":
norm = Normalize(cmin, cmax)
ax.set_ylabel(kwargs.get("ylabel", "f"))

im = ax.pcolormesh(xh, yh, image, cmap=cmap, norm=norm)

fig.colorbar(im, ax=ax)

if kwargs.get("kde", False) is True:
import seaborn as sns
Expand All @@ -105,7 +166,7 @@

ax.set_title(kwargs.get("title", ""))
ax.set_xlabel(kwargs.get("xlabel", axis[0]))
ax.set_ylabel(kwargs.get("ylabel", axis[1]))

if "xlim" in kwargs:
ax.set_xlim(kwargs["xlim"])
if "ylim" in kwargs:
Expand All @@ -116,15 +177,15 @@
if axis[0] in vaxis:
ax.axvline(
np.average(
particles.v[:, vaxis[axis[0]]], weights=particles.weights
new_particles.v[:, vaxis[axis[0]]], weights=new_particles.weights
),
color="w",
ls="--",
)
if axis[1] in vaxis:
ax.axhline(
np.average(
particles.v[:, vaxis[axis[1]]], weights=particles.weights
new_particles.v[:, vaxis[axis[1]]], weights=new_particles.weights
),
color="w",
ls="--",
Expand Down Expand Up @@ -331,8 +392,8 @@

ax.set_title(kwargs.get("title", ""))

ax.set_xlabel(kwargs.get("xlabel", "$x c / \omega_p$"))
ax.set_ylabel(kwargs.get("ylabel", "$y c / \omega_p$"))
ax.set_xlabel(kwargs.get("xlabel", "$x \\ (d_p)$"))
ax.set_ylabel(kwargs.get("ylabel", "$y \\ (d_p)$"))

if "xlim" in kwargs:
ax.set_xlim(kwargs["xlim"])
Expand Down
Loading