yt-project / yt

Main yt repository
http://yt-project.org
Other
469 stars 280 forks source link

BUG: broken documentation example for particle filters #3980

Open neutrinoceros opened 2 years ago

neutrinoceros commented 2 years ago

Bug report

Bug summary

The following notebook from yt's documentation is broken (raises a NotImplementedError) https://github.com/yt-project/yt/blob/main/doc/source/analyzing/particle_filter.ipynb

This was reported by Ivan Markin (@nogoegst) on Slack. Not unlike #3541, this seems like something that was broken long ago when we merged the 4.0 branch into main, so it's extremely difficult to bisect down. I'm also far from an expert in this region of the code, so I don't understand clearly what's happening and how to fix it. Did this happen because of an intentional API change ? or is this purely a regression in yt 4.0 ?

Code for reproduction Here's a self-contained version of the broken notebook

# broken docs example from
# 
# report on Slack by Ivan Markin
import yt
import numpy as np

ds = yt.load("TipsyGalaxy/galaxy.00300")

def young_stars(pfilter, data):
    age = data.ds.current_time - data[pfilter.filtered_type, "creation_time"]
    filter = np.logical_and(age.in_units('Myr') <= 5, age >= 0)
    return filter

def old_stars(pfilter, data):
    age = data.ds.current_time - data[pfilter.filtered_type, "creation_time"]
    filter = np.logical_or(age.in_units('Myr') >= 5, age < 0)
    return filter

yt.add_particle_filter("young_stars", function=young_stars, filtered_type='Stars', requires=["creation_time"])
yt.add_particle_filter("old_stars", function=old_stars, filtered_type='Stars', requires=["creation_time"])

ds.add_particle_filter('young_stars')
ds.add_particle_filter('old_stars')

p = yt.ProjectionPlot(ds, 'z', [('deposit', 'young_stars_cic'), ('deposit', 'old_stars_cic')], width=(40, 'kpc'), center='m')
p.set_figure_size(5)
p.save("/tmp/particle_filters.png")

Actual outcome

Traceback (most recent call last):
  File "/Users/robcleme/dev/yt-project/yt/t_docs.py", line 25, in <module>
    p = yt.ProjectionPlot(ds, 'z', [('deposit', 'young_stars_cic'), ('deposit', 'old_stars_cic')], width=(40, 'kpc'), center='m')
  File "/Users/robcleme/dev/yt-project/yt/yt/visualization/plot_window.py", line 2251, in __init__
    PWViewerMPL.__init__(
  File "/Users/robcleme/dev/yt-project/yt/yt/visualization/plot_window.py", line 919, in __init__
    PlotWindow.__init__(self, *args, **kwargs)
  File "/Users/robcleme/dev/yt-project/yt/yt/visualization/plot_window.py", line 270, in __init__
    self._setup_plots()
  File "/Users/robcleme/dev/yt-project/yt/yt/visualization/plot_window.py", line 1126, in _setup_plots
    image = self.frb[f]
  File "/Users/robcleme/dev/yt-project/yt/yt/visualization/fixed_resolution.py", line 167, in __getitem__
    buff = self.ds.coordinates.pixelize(
  File "/Users/robcleme/dev/yt-project/yt/yt/geometry/coordinates/cartesian_coordinates.py", line 222, in pixelize
    return self._ortho_pixelize(
  File "/Users/robcleme/dev/yt-project/yt/yt/geometry/coordinates/cartesian_coordinates.py", line 542, in _ortho_pixelize
    data_source["px"],
  File "/Users/robcleme/dev/yt-project/yt/yt/data_objects/data_containers.py", line 259, in __getitem__
    self.field_data[f] = self.ds.arr(self._generate_container_field(f))
  File "/Users/robcleme/dev/yt-project/yt/yt/data_objects/data_containers.py", line 426, in _generate_container_field
    raise NotImplementedError
NotImplementedError

Expected outcome This is the outcome in the last working version of the docs (3.6.1) (https://yt-project.org/docs/3.6.1/analyzing/filtering.html) Unknown

Version Information

unkaktus commented 2 years ago

Hi! @neutrinoceros thanks for opening the issue with such an extensive amount of details!

I was pondering with this problem for a while. It seems that the error occurs for _container_fields (data_containers.py), which happen to be px, py, etc. They are seemingly coming from the axis-aligned projections, so I have replaced ProjectionPlot(ds, "z", ...) call with OffAxisProjectionPlot(dataset, np.array([0.0, 0.0, 1.0]), ...), and it worked as expected.

I didn't expect ProjectionPlot to not be a special case of OffAxisProjectionPlot, but the underlying code seems to be different (also from different executions times).

Hope this helps!

neutrinoceros commented 2 years ago

Oh, interesting ! Indeed, the implementations for AxisAligned VS OffAxis plot classes are very different. Good job finding a workaround, that'll certainly help tracking the bug down.