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] volume rendering: sigma_clip normalization #3984

Merged
merged 4 commits into from
Jun 23, 2022

Conversation

chrishavlin
Copy link
Contributor

This closes #3979 and fixes an unreported bug in which not providing a sigma_clip value while trying to save a non-png results in a crash.

There were several spots where sigma_clip was used to calculated a clipping value, the version in save_annotated incorrectly included the alpha channel in the calculation. I added a ImageArray._clipping_value method to cut down on some of the duplicate code.

Here's a script based on the cookbook example mentioned in #3979

import yt
ds = yt.load("Enzo_64/DD0043/data0043")
sc = yt.create_scene(ds, lens_type="perspective")
source = sc[0]
source.set_field(("gas", "density"))
source.set_log(True)
# Set up the camera parameters: focus, width, resolution, and image orientation
sc.camera.focus = ds.domain_center
sc.camera.resolution = 800
sc.camera.north_vector = [0, 0, 1]
sc.camera.position = [1.7, 1.7, 1.7]
sc.annotate_axes(alpha=0.02)
sc.annotate_domain(ds, color=[1, 1, 1, 0.01])
text_string = f"T = {float(ds.current_time.to('Gyr'))} Gyr"

sc.save_annotated("vol_annotated_6.png", sigma_clip=6, text_annotate=[[(0.1, 0.95), text_string]])
sc.save("vol_save_6.png", sigma_clip=6, render=False)
sc.save("vol_save_6.jpg", sigma_clip=6, render=False)
sc.save("vol_save.jpg", render=False)  # this one errors on main!

The save_annotated and save versions now match:

vol_annotated_6
vol_save_6

Comment on lines -514 to -515
nz = im[im > 0.0]
nim = im / (nz.mean() + sigma_clip * np.std(nz))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just a note to reviewers: this was the source of the bug. It should only normalize based on the RGB channels (i.e., im[:,:,:3])

Copy link
Member

Choose a reason for hiding this comment

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

awesome. Nice catch. I never got around to digging deeper, so I'll happily review your PR ;)

zingale
zingale previously approved these changes Jun 22, 2022
@zingale
Copy link
Member

zingale commented Jun 22, 2022

I confirmed the output that this works as expected. Nice work @chrishavlin !!

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.

Nice job ! Found one caveat that can be fixed easily. I’ll merge when it’s dealt with

yt/visualization/volume_rendering/scene.py Outdated Show resolved Hide resolved
Co-authored-by: Clément Robert <cr52@protonmail.com>
@neutrinoceros neutrinoceros merged commit 32d923c into yt-project:main Jun 23, 2022
neutrinoceros added a commit to neutrinoceros/yt that referenced this pull request Aug 8, 2022
…lipping

[BUG] volume rendering: sigma_clip normalization
@neutrinoceros neutrinoceros added this to the 4.0.5 milestone Aug 9, 2022
@chrishavlin chrishavlin deleted the normalize_sigma_clipping branch June 14, 2023 18:51
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.

sigma_clip doesn't seem to work with save_annotated()
3 participants