search

yt-project / yt

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

OffAxisProjection wrong units for new fields #1986

Closed chummels closed 6 years ago

chummels commented 6 years ago

Bug report

Bug summary

Particle-based Off-Axis Projections give incorrect results for newly created derived fields that aren't simple aliases to on-disk fields. This is related to Issue #1906 , but didn't get addressed by its fix in PR #1961 .

Below is an example which demonstrates this problem. I create a "test" field, which is just a derived field identical to the ('gas', 'density') field and create an off-axis projection of it. As you can see, the images are the same, but the zlim is off by ~avogadro's number.

Code for reproduction

import yt
import trident
ds = yt.load('FIRE_M12i_ref11/snapshot_600.hdf5')
_, c = ds.find_max(('gas', 'density'))

# Add new field manually and OAP shows its units down by a factor of Avogadro's number
def _test(field, data):
    return data[('gas', 'density')]
ds.add_field(('gas', 'test'), function=_test, units="g*cm**-3", sampling_type='local')
yt.OffAxisProjectionPlot(ds, [0, 0, 1], ('gas', 'test'), center=c, width=(200, 'kpc')).save('off_test.png')
yt.OffAxisProjectionPlot(ds, [0, 0, 1], ('gas', 'density'), center=c, width=(200, 'kpc')).save('off_dens.png')

Actual outcome

Off-Axis Projection for new "test" field (off_test.png): off_test

Off-Axis Projection for density field (off_dens.png): off_dens

Expected outcome

Plots should match in all aspects including zscale.

Discussion

Digging into this, it appears that the problem lies in a mismatch as to how native and derived fields point to the SmoothingLength field. For example, if you investigate the contents of this line in the off_axis_projection machinery:

https://github.com/yt-project/yt/blob/yt-4.0/yt/visualization/volume_rendering/off_axis_projection.py#L207

by adding an additional line to just print out the information about the smoothing_length field:

print(data_source.ds._get_field_info('smoothing_length'))

It reveals the difference between these two cases. In the iteration with the native ('gas', 'density') field, we see this output:

Alias Field for "('PartType0', 'SmoothingLength')" (PartType0, smoothing_length): (units: code_length, particle field)

But in the case with our derived field ('gas', 'test'), we see this output:

Alias Field for "('PartType0', 'smoothing_length')" (gas, smoothing_length): (units: cm, particle field)

The difference is simply that the derived field ('gas', 'test') is pointing to the ('PartType0', 'smoothing_length') derived field, whereas ('gas', 'density') is pointing to the native field ('PartType0', 'SmoothingLength'), and those two fields have different units, which is throwing off the OAP unit normalization implemented in PR #1961.

I'm not sure how to address this, as I'm not sure why newly derived fields look at different smoothing_length fields than others do.

chummels commented 6 years ago

Fixed with PR #1987 .