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

ENH: add support for VelocityCallback and MagFieldCallBack in spherical coordinates #3817

Merged

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented Feb 18, 2022

PR Summary

minimal demo

import numpy as np
import yt
from unyt import cm, s

shape = (32, 128, 1)
a = np.sin(np.linspace(-np.pi/2, np.pi/2, 128))
b = np.ones((32, 128))
c = np.zeros((32, 128))

data = {
    ("gas", "velocity_r"): np.reshape(a * b, shape) * cm/s,
    ("gas", "velocity_theta"): np.reshape(a * c, shape) * cm/s,
}

ds = yt.load_uniform_grid(
    data,
    shape,
    bbox=np.array([[0.0, 5.0], [np.pi / 2 - 0.5, np.pi / 2 + 0.5], [-0.1, +0.1]]),
    geometry="spherical",
)

p = yt.SlicePlot(ds, "phi", "velocity_r")
p.annotate_velocity()
p.save("/tmp/")

UniformGridData_Slice_phi_velocity_r

image actualized on 2022-08-01

older version

UniformGridData_Slice_phi_velocity_r

note that the quiver plot is drawn well outside of the actual data domain. I think this effect is due to the current implementation of the pixelizer routine (pixelize_cylinder), combined with the "low resolution" buffer associated with the quiver plot for readability. I consider this a bug, but it's out of scope here.
Edit: this is solved in #3818

@neutrinoceros neutrinoceros force-pushed the add_spherical_velocity_callback branch 3 times, most recently from 54ec49b to b068bfd Compare February 18, 2022 21:15
@neutrinoceros neutrinoceros changed the title ENH: add support for VelocityCallback in spherical coordinates (poloidal cut) ENH: add support for VelocityCallback and MagFieldCallBack in spherical coordinates (poloidal cut) Feb 18, 2022
@neutrinoceros neutrinoceros force-pushed the add_spherical_velocity_callback branch from b068bfd to f553d0a Compare February 19, 2022 10:12
@neutrinoceros
Copy link
Member Author

neutrinoceros commented Feb 19, 2022

While I'm at it, I'll add support for "conic" projections too (theta-normal), illustrated here with a purely azimuthal magnetic field

import numpy as np
import yt
from yt.units import G

shape = (32, 128, 128)
a = np.sin(np.linspace(-np.pi / 2, np.pi / 2, 128))
b = np.ones((32, 128, 128))
c = np.zeros((32, 128, 128))

data = {
    ("gas", "magnetic_field_r"): np.reshape(a * c, shape) * G,
    ("gas", "magnetic_field_theta"): np.reshape(a * c, shape) * G,
    ("gas", "magnetic_field_phi"): np.reshape(a * b, shape) * G,
}

ds = yt.load_uniform_grid(
    data,
    shape,
    bbox=np.array([[1, 5], [np.pi / 2 - 0.5, np.pi / 2 + 0.5], [0, +np.pi]]),
    geometry="spherical",
)

p = yt.SlicePlot(ds, "theta", "magnetic_field_phi")
p.annotate_magnetic_field()
p.save("/tmp/")

UniformGridData_Slice_theta_magnetic_field_phi

image actualized on 2022-08-01

older version

UniformGridData_Slice_theta_magnetic_field_phi

@neutrinoceros neutrinoceros changed the title ENH: add support for VelocityCallback and MagFieldCallBack in spherical coordinates (poloidal cut) ENH: add support for VelocityCallback and MagFieldCallBack in spherical coordinates Feb 19, 2022
@neutrinoceros neutrinoceros force-pushed the add_spherical_velocity_callback branch 3 times, most recently from 9536841 to 68273d6 Compare February 19, 2022 14:57
@neutrinoceros
Copy link
Member Author

Tests exploding everywhere, switching to draft for now.

@neutrinoceros neutrinoceros marked this pull request as draft February 19, 2022 15:29
@neutrinoceros neutrinoceros force-pushed the add_spherical_velocity_callback branch from 68273d6 to b46569c Compare February 19, 2022 15:32
@neutrinoceros neutrinoceros marked this pull request as ready for review February 19, 2022 16:44
@neutrinoceros neutrinoceros added this to the 4.1.0 milestone Feb 19, 2022
@neutrinoceros neutrinoceros force-pushed the add_spherical_velocity_callback branch 2 times, most recently from 2a709a0 to c634192 Compare February 20, 2022 16:06
@matthewturk
Copy link
Member

I think your diagnosis -- the buffer being low-resolution -- is the root cause, but perhaps it can be fixed by more carefully adjusting the arguments to the pixelization routine to query at the specific pixel centers.

@matthewturk
Copy link
Member

I did not realize it depended on yt.mods. Oof! Also, I could be wrong about the mag field.

@neutrinoceros
Copy link
Member Author

I think your diagnosis -- the buffer being low-resolution -- is the root cause, but perhaps it can be fixed by more carefully adjusting the arguments to the pixelization routine to query at the specific pixel centers.

How about the proposed fix in #3818 ?

matthewturk
matthewturk previously approved these changes May 16, 2022
Copy link
Member

@matthewturk matthewturk left a comment

Choose a reason for hiding this comment

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

LGTM

yt/fields/vector_operations.py Show resolved Hide resolved
yt/fields/vector_operations.py Outdated Show resolved Hide resolved
@neutrinoceros neutrinoceros marked this pull request as draft May 17, 2022 11:10
@neutrinoceros
Copy link
Member Author

I need to revisit this. Switching to draft in the mean time. For now I'll just resolve a conflict and add a comment as requested by Matt.

@neutrinoceros neutrinoceros force-pushed the add_spherical_velocity_callback branch from 5ba0e53 to 581eb28 Compare May 17, 2022 11:43
@neutrinoceros
Copy link
Member Author

refreshing CI

@neutrinoceros neutrinoceros reopened this May 26, 2022
@neutrinoceros neutrinoceros force-pushed the add_spherical_velocity_callback branch from 581eb28 to 6035e9a Compare July 13, 2022 14:36
@neutrinoceros
Copy link
Member Author

neutrinoceros commented Jul 13, 2022

rebased to resolve conflicts and refresh CI

@neutrinoceros neutrinoceros force-pushed the add_spherical_velocity_callback branch 2 times, most recently from c7920e5 to 9b6c9f2 Compare July 15, 2022 15:10
@neutrinoceros
Copy link
Member Author

neutrinoceros commented Jul 15, 2022

this is now rebased on top of #4021. I also cleaned up a couple redundant definitions as previously discussed, and fixed a couple outstanding bugs.

@neutrinoceros
Copy link
Member Author

@yt-fido test this please

@neutrinoceros neutrinoceros changed the title ENH: add support for VelocityCallback and MagFieldCallBack in spherical coordinates ENH: add support for VelocityCallback and MagFieldCallBack in spherical coordinates (wait on #4021) Jul 30, 2022
@neutrinoceros neutrinoceros force-pushed the add_spherical_velocity_callback branch from 9b6c9f2 to 924f4f6 Compare August 1, 2022 18:01
@neutrinoceros neutrinoceros changed the title ENH: add support for VelocityCallback and MagFieldCallBack in spherical coordinates (wait on #4021) ENH: add support for VelocityCallback and MagFieldCallBack in spherical coordinates Aug 1, 2022
@neutrinoceros neutrinoceros force-pushed the add_spherical_velocity_callback branch from 924f4f6 to 000425a Compare August 1, 2022 20:09
@neutrinoceros neutrinoceros marked this pull request as ready for review August 1, 2022 21:04
@neutrinoceros
Copy link
Member Author

@matthewturk can we merge this ?

@matthewturk matthewturk merged commit 4c2603d into yt-project:main Aug 15, 2022
@neutrinoceros neutrinoceros deleted the add_spherical_velocity_callback branch August 15, 2022 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants