yt icon indicating copy to clipboard operation
yt copied to clipboard

OffAxisSlicePlot does not appear to work with particle-based datasets

Open chummels opened this issue 3 years ago • 11 comments

Bug report

Bug summary

When I attempt to generate an OffAxisSlicePlot for any particle-based datasets, I get unindexed type errors. The same code snippet is in our documentation for working with grid-based datasets and it works fine for those.

Code for reproduction

import yt
ds = yt.load_sample("GadgetDiskGalaxy")
yt.SlicePlot(ds, [1,1,0], ("gas", "density")).save()

Actual outcome

Traceback (most recent call last):
  File "test_slice2.py", line 3, in <module>
    yt.SlicePlot(ds, [1,1,0], ("gas", "density")).save()
  File "/Users/chummels/src/yt/yt/visualization/plot_window.py", line 2474, in SlicePlot
    return OffAxisSlicePlot(ds, normal, fields, *args, **kwargs)
  File "/Users/chummels/src/yt/yt/visualization/plot_window.py", line 1985, in __init__
    PWViewerMPL.__init__(
  File "/Users/chummels/src/yt/yt/visualization/plot_window.py", line 872, in __init__
    PlotWindow.__init__(self, *args, **kwargs)
  File "/Users/chummels/src/yt/yt/visualization/plot_window.py", line 256, in __init__
    self._setup_plots()
  File "/Users/chummels/src/yt/yt/visualization/plot_window.py", line 964, in _setup_plots
    self._recreate_frb()
  File "/Users/chummels/src/yt/yt/visualization/plot_window.py", line 316, in _recreate_frb
    self._frb._get_data_source_fields()
  File "/Users/chummels/src/yt/yt/visualization/fixed_resolution.py", line 176, in _get_data_source_fields
    self[f]
  File "/Users/chummels/src/yt/yt/visualization/fixed_resolution.py", line 139, in __getitem__
    buff = self.ds.coordinates.pixelize(
  File "/Users/chummels/src/yt/yt/geometry/coordinates/cartesian_coordinates.py", line 221, in pixelize
    return self._oblique_pixelize(data_source, field, bounds, size, antialias)
  File "/Users/chummels/src/yt/yt/geometry/coordinates/cartesian_coordinates.py", line 552, in _oblique_pixelize
    indices = np.argsort(data_source["pdx"])[::-1].astype(np.int_)
  File "/Users/chummels/src/yt/yt/data_objects/data_containers.py", line 255, in __getitem__
    self.field_data[f] = self.ds.arr(self._generate_container_field(f))
  File "/Users/chummels/src/yt/yt/data_objects/selection_objects/slices.py", line 270, in _generate_container_field
    return self._current_chunk.fwidth[:, 0] * 0.5
  File "/Users/chummels/src/yt/yt/geometry/geometry_handler.py", line 255, in cached_func
    tr = self._accumulate_values(n[1:])
  File "/Users/chummels/src/yt/yt/geometry/geometry_handler.py", line 292, in _accumulate_values
    arrs.append(f(self.dobj))
  File "/Users/chummels/src/yt/yt/data_objects/index_subobjects/particle_container.py", line 17, in _func_non_indexed
    raise YTNonIndexedDataContainer(self)
yt.utilities.exceptions.YTNonIndexedDataContainer: The data container (<class 'yt.data_objects.index_subobjects.particle_container.ParticleContainer'>) is an unindexed type.  Operations such as ires, icoords, fcoords and fwidth will not work on it.

Expected outcome

I expected it to produce an OffAxisSlicePlot.

Version Information

current tip of yt dev: 88b16fbbe889

chummels avatar Sep 16 '21 06:09 chummels

Does OffAxisProjectionPlot support particle datasets ?

neutrinoceros avatar Sep 16 '21 06:09 neutrinoceros

Yes--I use it all the time.

chummels avatar Sep 16 '21 07:09 chummels

Ok then should we deal with https://github.com/yt-project/yt/pull/3489 before or after fixing this ?

neutrinoceros avatar Sep 16 '21 07:09 neutrinoceros

AFAIK we haven't supported this yet at all (probably an oversight in releasing 4.0)

jzuhone avatar Sep 16 '21 11:09 jzuhone

yeah so I'm hesitant to call this a "bug", rather a missing feature ? Though the fact that you hit a YTNonIndexedDataContainer instead of a NotImplementedError is a bug in itself, IMO. I'd advise adding a "bug" and a "new feature" label here.

neutrinoceros avatar Sep 16 '21 12:09 neutrinoceros

It appears that you're right @jzuhone as per the message sort of buried here in the narrative docs: https://yt-project.org/docs/dev/visualizing/plots.html#additional-notes-for-plotting-particle-data (thanks, @brittonsmith for the link). Given that, I think we should update the docstring for OffAxisSlicePlot to say that it doesn't work with particle-based datasets and change the error to NotImplemented, but in an ideal world we would just implement it the code to work with particles, since this is sort of core-functionality.

chummels avatar Sep 16 '21 14:09 chummels

I think we should update the docstring for OffAxisSlicePlot to say that it doesn't work with particle-based datasets and change the error to NotImplemented, but in an ideal world we would just implement it the code to work with particles, since this is sort of core-functionality.

Unless anyone has time to implement it soon, how about we do the easy fix (docstring + NotImplementedError) now and keep this issue open within the 4.1 milestone so it is at least discussed (addressed ?) in time for the next feature release ?

neutrinoceros avatar Sep 16 '21 15:09 neutrinoceros

Yeah, that works for me. I am not sure how to go about implementing this to be honest, but I do think it's important that we address this at some point since we're actively courting particle-based simulation users with the release of yt4.

chummels avatar Sep 16 '21 19:09 chummels

Agreed. Can you make a PR with docs + the corrected error ?

neutrinoceros avatar Sep 16 '21 20:09 neutrinoceros

related to #2628

neutrinoceros avatar Oct 23 '21 15:10 neutrinoceros

note that, while the example still fails, the error message was intentionally changed in #4038

neutrinoceros avatar Aug 22 '22 22:08 neutrinoceros