Proposed change to how YTArray handles sums and differences with zero
by Nathan Goldbaum
Hi all,
Today a student I am working with ran into an issue illustrated by the
following test script:
from yt.units import cm
arr = [1, 2, 3]*cm
sum(arr)
Right now this errors out with the following traceback:
http://paste.yt-project.org/show/6378/
The reason for this is that the python builtin sum command has an optional
`start` argument, which defaults to 0. The error is happening because the
first element of the array (1 km) has different units from 0.
Sum is probably not the best way to do this, but I think erroring out here
is an overly harsh user experience, especially for a user who is new to
python and might not be aware of the possible performance implications of
using sum on a large array.
Incidentally, an analogous script with astropy quantities does work:
from astropy.units import cm
arr = [1, 2, 3]*cm
sum(arr)
This is because astropy's quantities let you add zero (and just zero, no
other float or integer will work) to an array or quantity with any units.
On a philosophical level, the difference between 0 cm and 0 is not
particular useful in practice, and operationally allowing 0 to be added or
subtracted from YTArray instances without erroring out should not have any
concrete implications for real programs, since until now it has raised an
error. To be honest, I'm now regretting not thinking carefully about this
until now....
Since this is a change to how YTArray, which is fundamental to yt's
internals, I wanted to check with the list before proposing a pull request
that implements this change.
Does anyone have any objections?
Thanks!
-Nathan
2 years, 8 months
Issue #1203: Incorrect particle_velocity_cylindrical_theta and particle_velocity_cylindrical_radius when "center" != [0, 0, 0] (yt_analysis/yt)
by Ji-hoon Kim
New issue 1203: Incorrect particle_velocity_cylindrical_theta and particle_velocity_cylindrical_radius when "center" != [0, 0, 0]
https://bitbucket.org/yt_analysis/yt/issues/1203/incorrect
Ji-hoon Kim:
Dear All,
The current implementations of "particle_velocity_cylindrical_theta" and "particle_velocity_cylindrical_radius" seem to incorrectly calculate "theta" when "center" is not at [0, 0, 0]. As an example, for a rotating stellar disk, currently the ParticleProjectionPlot() of "particle_velocity_cylindrical_theta" produces the following (incorrect) image:
Note that the center in this particular dataset (ARTIO) is not at [0,0,0], but at ~[64, 64, 64] in a [0,0,0] - [128, 128, 128] box (in code_length units). On the other hand, if you move "theta = get_cyl_theta(pos, normal)" a couple of lines down after "center" is subtracted out of "pos":
```
#!python
def _particle_velocity_cylindrical_theta(field, data):
"""The cylindrical theta component of the particle velocities
Relative to the coordinate system defined by the *normal* vector,
*bulk_velocity* vector and *center* field parameters.
"""
normal = data.get_field_parameter('normal')
center = data.get_field_parameter('center')
bv = data.get_field_parameter("bulk_velocity")
pos = data.ds.arr([data[ptype, spos % ax] for ax in "xyz"])
vel = data.ds.arr([data[ptype, svel % ax] for ax in "xyz"])
# theta = get_cyl_theta(pos, normal)
pos = pos - np.reshape(center, (3, 1))
vel = vel - np.reshape(bv, (3, 1))
theta = get_cyl_theta(pos, normal) ##### <=== HERE!
cylt = get_cyl_theta_component(vel, theta, normal)
return cylt
```
Then, the same ParticleProjectionPlot() creates the following (correct) image:
It would be great if someone could independently check this error and the proposed solution. Any information or insight will be appreciated. Thank you as always to the yt community!
Best regards,
Ji-hoon
2 years, 8 months
Unusually commit traffic today
by Nathan Goldbaum
Hi all,
Today I prematurely merged the pull request that adds the new default
colormap. This was on me for not being careful to make sure the tests were
passing before hitting the merge button.
Kacper helpfully suggested on Slack that the yt-fido bot should be able to
"approve" pull requests when the tests pass. This should help us more
easily see that a pull request is passing the tests just by glancing at the
header for the pull request. I've opened an issue to track this here:
https://bitbucket.org/yt_analysis/yt/issues/1201/fido-bot-should-approve-...
The tests were broken for an hour or so, but were fixed by merging PR 2090,
so hopefully the disruption was minimal. Sorry for the mistake - I will try
to be more conscientious about merging pull requests going forward.
-Nathan
2 years, 8 months
Issue #1198: Add ability to plot cell edges (yt_analysis/yt)
by Matthew Turk
New issue 1198: Add ability to plot cell edges
https://bitbucket.org/yt_analysis/yt/issues/1198/add-ability-to-plot-cell...
Matthew Turk:
Right now we can plot grid patch edges with `annotate_grids`, but not the cell edges.
This should be straightforward to implement with a flag in the call to the pixelizer. Much like how the line pixelization occurs in the volume renderer, when a pixel is near to the edge of a cell (comparing `pdx` `pdy` `px` and `py`) closer than some epsilon (user specifiable, probably defaulting to something sane and thin) it should toss in a black line. Since the pixelization right now works as returning data values, this would be a separate call that would return RGBA values where the color is either (0.0, 0.0, 0.0, 1.0) or (0.0, 0.0, 0.0, 0.0).
Responsible: MatthewTurk
2 years, 8 months
