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

Adds data_source argument to covering grid #4063

Merged
merged 8 commits into from
Sep 7, 2022

Conversation

chrishavlin
Copy link
Contributor

This adds a data_source keyword (only) argument to the YTCoveringGrid and then intersects that data_source with the region used for the covering grid in order to create a regularized sampling of, e.g., a sphere.

Here's an example:

import yt
import numpy as np
import matplotlib.pyplot as plt 

ds = yt.load_sample("IsolatedGalaxy")

cg = ds.covering_grid(2, left_edge=[0.0, 0.0, 0.0], dims=[128, 128, 128])
gridded_dens = cg[("gas", "density")]

sp = ds.sphere(ds.domain_center, 0.2)
cg_sp_intx = ds.covering_grid(2, left_edge=[0.0, 0.0, 0.0], dims=[128, 128, 128], data_source=sp)
gridded_dens_in_sphere = cg_sp_intx[("gas", "density")]

f, axs = plt.subplots(nrows=1, ncols=2)
axs[0].imshow(np.log10(gridded_dens[:,:,64]))
axs[1].imshow(np.log10(gridded_dens_in_sphere[:,:,64]))

image

Pushing a nominally working draft now... but when ready this would close #4052

@chrishavlin chrishavlin added enhancement Making something better new feature Something fun and new! labels Aug 8, 2022
matthewturk
matthewturk previously approved these changes Aug 8, 2022
yt/data_objects/construction_data_containers.py Outdated Show resolved Hide resolved
yt/data_objects/construction_data_containers.py Outdated Show resolved Hide resolved
@chrishavlin
Copy link
Contributor Author

chrishavlin commented Aug 8, 2022

note on current state: no new tests, a new example in the documentation would be good.

@chrishavlin
Copy link
Contributor Author

edit: added tests and updated docs. Going to switch to ready-to-review. I won't be around for a little while but feel free to review!

@chrishavlin chrishavlin marked this pull request as ready for review August 9, 2022 22:21
@neutrinoceros neutrinoceros self-requested a review August 17, 2022 15:22
@neutrinoceros
Copy link
Member

refreshing CI. I will review this over the weekend

@neutrinoceros
Copy link
Member

@yt-fido test this please

1 similar comment
@neutrinoceros
Copy link
Member

@yt-fido test this please

Copy link
Member

@neutrinoceros neutrinoceros left a comment

Choose a reason for hiding this comment

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

Nothing major, but I do have a couple requests. Overall, I really like this.

yt/data_objects/construction_data_containers.py Outdated Show resolved Hide resolved
yt/data_objects/construction_data_containers.py Outdated Show resolved Hide resolved
yt/data_objects/construction_data_containers.py Outdated Show resolved Hide resolved
yt/data_objects/tests/test_covering_grid.py Outdated Show resolved Hide resolved
chrishavlin and others added 2 commits September 7, 2022 10:38
Copy link
Member

@neutrinoceros neutrinoceros left a comment

Choose a reason for hiding this comment

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

I see @matthewturk previously approved this but I'll hold off merging until he signs off again.

@matthewturk matthewturk merged commit 74adc7e into yt-project:main Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Making something better new feature Something fun and new!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: Allow YTCoveringGrid to accept a data source
3 participants