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

BUG: Moirés in cylindrical pixelization #3781

Closed
neutrinoceros opened this issue Feb 6, 2022 · 1 comment · Fixed by #3782
Closed

BUG: Moirés in cylindrical pixelization #3781

neutrinoceros opened this issue Feb 6, 2022 · 1 comment · Fixed by #3782

Comments

@neutrinoceros
Copy link
Member

Bug report

Bug summary

For datasets in spherical coordinates, plotting a poloidal slice (i.e in the "R, z", "phi"-normal plane) can produce very visible moirés patterns. This effect seems to be strongest when the latitudinal domain is narrow, and not noticeable for sufficiently large domains.

Code for reproduction

import numpy as np
import yt

shape = (128, 128, 2)
data = {("gas", "density"): np.random.random_sample(shape)}
for theta_min in (0, 1.3, np.pi / 2 - 0.1):
    ds = yt.load_uniform_grid(
        data,
        shape,
        bbox=np.array([[0.5, 5.0], [theta_min, np.pi / 2 + 0.1], [-0.1, +0.1]]),
        geometry="spherical",
    )

    p = yt.SlicePlot(ds, "phi", "density")
    p.save(f"/tmp/phi_slice_{theta_min=}.png")

Going from widest to narrowest theta domains

Actual outcome
phi_slice_theta_min=0
phi_slice_theta_min=1 3
phi_slice_theta_min=1 5

This problem appears on yt 4.0.2 and on the dev branch, but it seems to be absent with 4.0.1 version, which was buggy in a different way (plot limits are not being calculated correctly)

phi_slice_yt-4 0 1_theta_min=0
phi_slice_yt-4 0 1_theta_min=1 3
phi_slice_yt-4 0 1_theta_min=1 4707963267948965

Precisely, the problem bisects to ea10a37 (#3533).

So I'm hesitant to call this a regression because the pixelizer routine wasn't changed in this commit, only the plot limits calculation was affected. I believe the patch only made this problem apparent but that it was already present in yt 4.0.1

A possibility I want to explore is that the pixelizer may be doing something wrong when pixels are far from being squares. Indeed the default buffer (800x800) size is most appropriate for square pictures. Right now it's a little hard to test because the SlicePlot.set_buff_size method seems to be broken too (it affects the plot limits but it shouldn't). I note however than the effect can be mitigated on the user side by reducing the buffer resolution, which still works for square buffers :

    p.set_buff_size((200, 200))
    p.save(f"/tmp/phi_slice_{theta_min=}_rect_buffer.png")

phi_slice_theta_min=1 4707963267948965_rect_buffer

this problem was first discovered downstream in neutrinoceros/yt_idefix#86

@neutrinoceros
Copy link
Member Author

Had a look at the pixelizer and found that the implementation may be excessively eager:
reducing what looks to me like an arbitrary jump as follow makes the problem disappear (for the default buffer resolution)

diff --git a/yt/utilities/lib/pixelization_routines.pyx b/yt/utilities/lib/pixelization_routines.pyx
index 18c35bc59..2d3571914 100644
--- a/yt/utilities/lib/pixelization_routines.pyx
+++ b/yt/utilities/lib/pixelization_routines.pyx
@@ -570,7 +570,7 @@ def pixelize_cylinder(np.float64_t[:,:] buff,
             continue
         theta_i = theta0 - dtheta_i
         # Buffer of 0.5 here
-        dthetamin = 0.5*dx/(r0 + dr_i)
+        dthetamin = 0.1*dx/(r0 + dr_i)
         while theta_i < theta0 + dtheta_i:
             r_i = r0 - dr_i
             costheta = math.cos(theta_i)
@@ -586,7 +586,7 @@ def pixelize_cylinder(np.float64_t[:,:] buff,
                 if pi >= 0 and pi < buff.shape[0] and \
                    pj >= 0 and pj < buff.shape[1]:
                     buff[pi, pj] = field[i]
-                r_i += 0.5*dx
+                r_i += 0.1*dx
             theta_i += dthetamin
import numpy as np
import yt

shape = (128, 128, 2)
data = {("gas", "density"): np.random.random_sample(shape)}
theta_min = np.pi / 2 - 0.1
ds = yt.load_uniform_grid(
    data,
    shape,
    bbox=np.array([[0.5, 5.0], [theta_min, theta_min+0.1], [-0.1, +0.1]]),
    geometry="spherical",
)

yt.SlicePlot(ds, "phi", "density").save("/tmp/")

Main branch

phi_slice_yt-4 1 dev0_theta_min=1 4707963267948965

With this patch
phi_slice_yt-4 1 dev0_theta_min=1 4707963267948965

The issue with this patch is that it is still arbitrarily eager and I expect it to break for high resolution images. It's also a performance regression for existing cases. I'm sure the correct solution is to tune these increments in a more robust way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant