PPV: Could not find field '('gas', 'v_los')' caused by PR #1176

yt-fido commented 10 years ago

Originally reported by: Kacper Kowalik (Bitbucket: xarthisius, GitHub: xarthisius)

Following script:

#!python

import yt
import numpy as np
from yt.analysis_modules.ppv_cube.api import PPVCube

nx, ny, nz = (64, 64, 64)
R = 10.
r_0 = 3.
beta = 1.4
alpha = -1.
x, y = np.mgrid[-R:R:nx * 1j, -R:R:ny * 1j]
r = np.sqrt(x * x + y * y)
theta = np.arctan2(y, x)

dens = np.zeros((nx, ny, nz))
dens[:, :, nz / 2 - 3:nz / 2 + 3] = (r ** alpha).reshape(nx, ny, 1)
vel_theta = r / (1. + (r / r_0) ** beta)
velx = np.zeros((nx, ny, nz))
vely = np.zeros((nx, ny, nz))
velx[:, :, nz / 2 - 3:nz / 2 + 3] = \
    (-vel_theta * np.sin(theta)).reshape(nx, ny, 1)
vely[:, :, nz / 2 - 3:nz / 2 + 3] = \
    (vel_theta * np.cos(theta)).reshape(nx, ny, 1)
dens[r > R] = 0.0
velx[r > R] = 0.0
vely[r > R] = 0.0

data = {"density": (dens, "g/cm**3"), "velocity_x": (velx, "km/s"),
        "velocity_y": (vely, "km/s"),
        "velocity_z": (np.zeros((nx, ny, nz)), "km/s")}
bbox = np.array([[-0.5, 0.5], [-0.5, 0.5], [-0.5, 0.5]])
ds = yt.load_uniform_grid(data, (nx, ny, nz),
                          length_unit=(2 * R, "kpc"), nprocs=1, bbox=bbox)

i = 60. * np.pi / 180.
L = [0.0, np.sin(i), np.sin(i)]

cube = PPVCube(ds, L, "density", dims=(
    100, 50, 25), velocity_bounds=(-1.5, 1.5, "km/s"))

yields:

#!text

Traceback (most recent call last):
  File "ala.py", line 38, in <module>
    100, 50, 25), velocity_bounds=(-1.5, 1.5, "km/s"))
  File "yt/analysis_modules/ppv_cube/ppv_cube.py", line 100, in __init__
    (self.nx, self.ny), "intensity")
  File "yt/visualization/volume_rendering/camera.py", line 2376, in off_axis_projection
    image = projcam.snapshot()
  File "yt/visualization/volume_rendering/camera.py", line 2301, in snapshot
    image, sampler),
  File "yt/visualization/volume_rendering/camera.py", line 2258, in _render
    data = [(grid[field] * mask).astype("float64") for field in fields]
  File "yt/data_objects/grid_patch.py", line 90, in __getitem__
    tr = super(AMRGridPatch, self).__getitem__(key)
  File "yt/data_objects/data_containers.py", line 240, in __getitem__
    self.get_data(f)
  File "yt/data_objects/data_containers.py", line 673, in get_data
    self._generate_fields(fields_to_generate)
  File "yt/data_objects/data_containers.py", line 693, in _generate_fields
    fd = self._generate_field(field)
  File "yt/data_objects/data_containers.py", line 277, in _generate_field
    tr = self._generate_fluid_field(field)
  File "yt/data_objects/data_containers.py", line 297, in _generate_fluid_field
    rv = finfo(gen_obj)
  File "yt/fields/derived_field.py", line 184, in __call__
    dd = self._function(self, data)
  File "yt/analysis_modules/ppv_cube/ppv_cube.py", line 159, in _intensity
    vlos = data["v_los"]
  File "yt/data_objects/grid_patch.py", line 90, in __getitem__
    tr = super(AMRGridPatch, self).__getitem__(key)
  File "yt/data_objects/data_containers.py", line 233, in __getitem__
    f = self._determine_fields([key])[0]
  File "yt/data_objects/data_containers.py", line 505, in _determine_fields
    raise YTFieldNotFound((ftype,fname),self.ds)
yt.utilities.exceptions.YTFieldNotFound: Could not find field '('gas', 'v_los')' in UniformGridData.

with current tip. Regression was introduced in changeset a9aa29ba3512

Bitbucket: https://bitbucket.org/yt_analysis/yt/issue/898

yt-fido commented 9 years ago

Original comment by Matt Turk (Bitbucket: MatthewTurk, GitHub: MatthewTurk):

Merged in drudd/yt (pull request #1217)

[Bugfix] ds.derived_field_list is often out of sync with ds.field_info [Fixes #898]

yt-fido commented 9 years ago

Original comment by Matt Turk (Bitbucket: MatthewTurk, GitHub: MatthewTurk):

Merged in drudd/yt (pull request #1217)

[Bugfix] ds.derived_field_list is often out of sync with ds.field_info [Fixes #898]

yt-fido commented 10 years ago

Original comment by chummels (Bitbucket: chummels, GitHub: chummels):

This is along similar lines of the question I asked a few days ago on another PR. I personally do not understand why we have a field_info object that is not always in sync with the derived_field_list. Perhaps it is not feasible, or there is a reason for this, but I like the idea that @drudd suggests of having the derived_field_list derived explicitly from the field_info.keys() in order to avoid things getting out of sync. Is there a reason for having two separate ways of cataloguing fields? I'm not asking to be a jerk--just because I don't understand this area of the code well.

yt-fido commented 10 years ago

Original comment by Douglas Rudd (Bitbucket: drudd, GitHub: drudd):

I don't fully understand the code, so some of these are probably not possible due to lazy-instantiation (either avoiding I/O or allowing fields to be defined out of order

don't keep derived_field_list around but generate it from field_info.keys() when needed.
add dependency checking to add_field rather than having check_derived_fields
if the above is not possible, have a flag for fields added after derived fields has been created, and in that case take the necessary steps to ensure consistency (both of fields and the lists)

yt-fido commented 10 years ago

Original comment by Matt Turk (Bitbucket: MatthewTurk, GitHub: MatthewTurk):

I'm really open to suggestions for improvement.

yt-fido commented 10 years ago

Original comment by Douglas Rudd (Bitbucket: drudd, GitHub: drudd):

The field handling code does not keep itself internally consistent, and because of that it's not clear how to uniquely check whether a field is defined. This particular instance, the add_field function that creates this field

#!python

        ds.field_info.add_field(("gas","v_los"), function=_vlos, units="cm/s")

does not add that field to ds.derived_fields. We can add another line to _determine_fields:

#!python

                # really ugly check to ensure that this field really does exist somewhere,
                # in some naming convention, before returning it as a possible field type
                if (ftype, fname) not in self.ds.field_info and \
                        (ftype,fname) not in self.ds.field_list and \
                        fname not in self.ds.field_list and \
                        (ftype,fname) not in self.ds.derived_field_list and \
                        fname not in self.ds.derived_field_list and \
                        (ftype,fname) not in self._container_fields:
                    raise YTFieldNotFound((ftype,fname),self.ds)

but it really seems that add_field should keep everything in sync.

yt-project / yt

PPV: Could not find field '('gas', 'v_los')' caused by PR #1176 #898