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: Fix off-axis particle projections with a weight field #4482

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

jzuhone
Copy link
Contributor

@jzuhone jzuhone commented Jun 8, 2023

PR Summary

The new off-axis particle projections currently fail when a weight_field is provided:

p = yt.ParticleProjectionPlot(ds, L, ("PartType4", "particle_velocity_z"), 
                              weight_field=("PartType4","particle_mass"))

gives:

---------------------------------------------------------------------------
File ~/Source/yt/yt/visualization/particle_plots.py:454, in ParticleProjectionPlot.__init__(self, ds, normal, fields, color, center, width, depth, weight_field, axes_unit, origin, fontsize, field_parameters, window_size, aspect, data_source, deposition, density, north_vector, axis)
    450     plt_origin = "center-window"
    452 self.projected = weight_field is None
--> 454 PWViewerMPL.__init__(
    455     self,
    456     ParticleSource,
    457     bounds,
    458     origin=plt_origin,
    459     fontsize=fontsize,
    460     fields=fields,
    461     window_size=window_size,
    462     aspect=aspect,
    463     splat_color=splat_color,
    464     geometry=ds.geometry,
    465     periodic=periodic,
    466     oblique=oblique,
    467 )
    469 self.set_axes_unit(axes_unit)
    471 if not self._use_cbar:

File ~/Source/yt/yt/visualization/plot_window.py:869, in PWViewerMPL.__init__(self, *args, **kwargs)
    867     self._plot_type = kwargs.pop("plot_type")
    868 self._splat_color = kwargs.pop("splat_color", None)
--> 869 PlotWindow.__init__(self, *args, **kwargs)
    871 # import type here to avoid import cycles
    872 # note that this import statement is actually crucial at runtime:
    873 # the filter methods for the present class are defined only when
    874 # fixed_resolution_filters is imported, so we need to guarantee
    875 # that it happens no later than instantiation
    876 from yt.visualization.plot_modifications import PlotCallback

File ~/Source/yt/yt/visualization/plot_window.py:253, in PlotWindow.__init__(self, data_source, bounds, buff_size, antialias, periodic, origin, oblique, window_size, fields, fontsize, aspect, setup, geometry)
    250     self._projection = get_mpl_transform(projection)
    251     self._transform = get_mpl_transform(transform)
--> 253 self._setup_plots()
    255 for field in self.data_source._determine_fields(self.fields):
    256     finfo = self.data_source.ds._get_field_info(field)

File ~/Source/yt/yt/visualization/plot_window.py:1071, in PWViewerMPL._setup_plots(self)
   1067     extenty.convert_to_units(unit_y)
   1069 extent = [*extentx, *extenty]
-> 1071 image = self.frb[f]
   1072 font_size = self._font_properties.get_size()
   1074 if f in self.plots.keys():

File ~/Source/yt/yt/visualization/fixed_resolution.py:836, in ParticleImageBuffer.__getitem__(self, item)
    834     weight_buff[weight_buff_mask == 0] = np.nan
    835     locs = np.where(weight_array > 0)
--> 836     ia[locs] /= weight_array[locs]
    838 self.data[item] = ia
    839 return self.data[item]

File ~/Source/unyt/unyt/array.py:1751, in unyt_array.__setitem__(self, item, value)
   1749 if hasattr(value, "units"):
   1750     if value.units != self.units and value.units != NULL_UNIT:
-> 1751         value = value.to(self.units)
   1752 super().__setitem__(item, value)

File ~/Source/unyt/unyt/array.py:967, in unyt_array.to(self, units, equivalence, **kwargs)
    925 def to(self, units, equivalence=None, **kwargs):
    926     """
    927     Creates a copy of this array with the data converted to the
    928     supplied units, and returns it.
   (...)
    965     898755178736817.6 J
    966     """
--> 967     return self.in_units(units, equivalence=equivalence, **kwargs)

File ~/Source/unyt/unyt/array.py:895, in unyt_array.in_units(self, units, equivalence, **kwargs)
    893 else:
    894     new_units = units
--> 895     (conversion_factor, offset) = self.units.get_conversion_factor(
    896         new_units, self.dtype
    897     )
    898 dsize = max(2, self.dtype.itemsize)
    899 if self.dtype.kind in ("u", "i"):

File ~/Source/unyt/unyt/unit_object.py:677, in Unit.get_conversion_factor(self, other_units, dtype)
    650 def get_conversion_factor(self, other_units, dtype=None):
    651     """Get the conversion factor and offset (if any) from one unit
    652     to another
    653 
   (...)
    675     (1.7999999999999998, -31.999999999999886)
    676     """
--> 677     return _get_conversion_factor(self, other_units, dtype)

File ~/Source/unyt/unyt/unit_object.py:922, in _get_conversion_factor(old_units, new_units, dtype)
    899 """
    900 Get the conversion factor between two units of equivalent dimensions. This
    901 is the number you multiply data by to convert from values in `old_units` to
   (...)
    919 
    920 """
    921 if old_units.dimensions != new_units.dimensions:
--> 922     raise UnitConversionError(
    923         old_units, old_units.dimensions, new_units, new_units.dimensions
    924     )
    925 old_basevalue = old_units.base_value
    926 old_baseoffset = old_units.base_offset

UnitConversionError: Cannot convert between 'code_velocity/code_mass' (dim '(length)/((mass)*(time))') and 'code_velocity' (dim '(length)/(time)').

The units were not being handled correctly in the augmented division operation. After looking at this, it seems as if we did not need to make the weight field's array a separate ImageArray instance since it's just thrown away anyway. This PR simplifies the code and fixes the bug.

PR Checklist

  • New features are documented, with docstrings and narrative docs
  • Adds a test for any bugs fixed. Adds tests for new features.

@jzuhone jzuhone added the bug label Jun 8, 2023
@neutrinoceros neutrinoceros added this to the 4.2.1 milestone Jun 8, 2023
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.

LGTM, thank you !

@matthewturk matthewturk merged commit 7d421b8 into yt-project:main Jun 8, 2023
meeseeksmachine pushed a commit to meeseeksmachine/yt that referenced this pull request Jun 8, 2023
@jzuhone jzuhone deleted the fix_offaxis_particle_weight branch January 22, 2024 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants