yt-project / yt

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

cylindrical_radial_velocity does not look right #926

Closed yt-fido closed 7 years ago

yt-fido commented 10 years ago

Originally reported by: Suoqing Ji (Bitbucket: jisuoqing, GitHub: jisuoqing)


Recently I just found that cylindrical_radial_velocity in yt3 does not give me what I expect. To test this, I setup a 3-D Sedov problem with domain [-0.5, 0.5]^3 and explosion center at [0, 0, 0], and plot the cylindrical_radial_velocity as the following image. According to the pattern of the velocity streamline, the cylindrical radial velocity should be positive.

sedov_hdf5_chk_0001_Slice_z_cylindrical_radial_velocity_original.png

I don't understand in the function of _cylindrical_radial in yt/fields/vector_operations.py:

#!python

    def _cylindrical_radial(field, data):
        normal = data.get_field_parameter("normal")
        vectors = obtain_rv_vec(data, (xn, yn, zn),
                                "bulk_%s" % basename)
        theta = resize_vector(data["index", 'cylindrical_theta'], vectors)
        return get_cyl_r_component(vectors, theta, normal)

what does the line of resize_vector do. If I replace this simply with line theta = data['cylindrical_theta'], it will give the image I expected as the following.

sedov_hdf5_chk_0001_Slice_z_cylindrical_radial_velocity_modified.png

In addition, if I want to plot the same field for a 2-D Sedov dataset, the error of yt.utilities.exceptions.YTFieldNotFound: Could not find field '('all', 'cylindrical_radial_velocity')' will be raised. However, if changing the line

#!python
vectors = obtain_rv_vec(data, (xn, yn, zn),
                                "bulk_%s" % basename)

into

#!python
vectors = obtain_rv_vec(data)

the correct image will be generated without raising any error. But I'm not very certain what's the correct way to fix this.


yt-fido commented 9 years ago

Original comment by Hilary Egan (Bitbucket: hegan, GitHub: hegan):


Resolved by PR #1311

yt-fido commented 10 years ago

Original comment by Benjamin Thompson (Bitbucket: cosmosquark, GitHub: cosmosquark):


@hegan @jisuoqing I am interested in what you find, was going to add cylindrical_velocities to the particle data too (cylindrical_radius for particles only exist in this PR for now) https://bitbucket.org/yt_analysis/yt/pull-request/1308/particle_cylindrical_r-and/diff . I imagine that the particles may have something wrong with the radial velocities as well.

yt-fido commented 10 years ago

Original comment by Nathan Goldbaum (Bitbucket: ngoldbaum, GitHub: ngoldbaum):


@hegan nope, I think this just needs someone who is willing to look at it.

yt-fido commented 10 years ago

Original comment by Hilary Egan (Bitbucket: hegan, GitHub: hegan):


This is also broken for regular "radial_velocity". Has any work been done to fix this? I can take this on if no one else has.

yt-fido commented 10 years ago

Original comment by Britton Smith (Bitbucket: brittonsmith, GitHub: brittonsmith):


Either calling resize vector or the function itself is wrong. It is resizing all incoming arrays to be of size (3, 1, 1, 1) or (3, 1). Either way, it seems to be chopping off all of the actual data.

yt-fido commented 10 years ago

Original comment by Britton Smith (Bitbucket: brittonsmith, GitHub: brittonsmith):


I had just been looking radius fields themselves, but these could have similar problems.

yt-fido commented 10 years ago

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


@brittonsmith does this look familiar to you? I recall you looked recently at the various radial velocities.