(This message is particularly relevant for Doug, Stephen and Nathan.)
I've been tracking down some failing tests. The script I have used is here:
Here are the four images:
They're both wrong. But, in related ways. Note that there's only *one* grid in the fake_random_pf, so the grids may also be getting selected wrong, but that's being masked here by the other problem.
v2 => The wrong cells are being selected. This is related to the v3 problem, as v2 uses the RadiusCode field to select cells that are within the data object.
v3 => The correct cells are being selected, but the problem with RadiusCode persists. Fortunately, we have a different cell-selection method. (I also think we may have too loose a restriction on cell selection in v3, but that's another topic; once we can sort out this script we can bring that up.)
So I think the ultimate problem here is RadiusCode, and underneath that, get_radius. I know I provided comments on this, but I am now having a hard time figuring out how to fix it. Here's the source:
def get_radius(data, field_prefix): center = data.get_field_parameter("center") DW = data.pf.domain_right_edge - data.pf.domain_left_edge radius = np.zeros(data[field_prefix+"x"].shape, dtype='float64') r = radius.copy() if any(data.pf.periodicity): rdw = radius.copy() for i, ax in enumerate('xyz'): np.subtract(data["%s%s" % (field_prefix, ax)], center[i], r) if data.pf.periodicity[i] == True: np.subtract(DW[i], r, rdw) np.abs(r, r) np.minimum(r, rdw, r) np.power(r, 2.0, r) np.add(radius, r, radius) np.sqrt(radius, radius) return radius
The periodicity seems to be the issue here. As a note, this actually does work with center on 0, 0, 0, which led me to realize it's an asymmetric problem of wrapping-right, and not wrapping-left. I *believe* this can be fixed by changing inside the periodicity check to be:
if data.pf.periodicity[i] == True: np.abs(r, r) np.subtract(r, DW[i], rdw) np.abs(rdw, rdw) np.minimum(r, rdw, r)
I have issued a pull request for this, but I felt it appropriate to email the list because I would like to make sure this issue is raised for a somewhat larger audience than those who get the PR notifications.