Fixing RAMSES density and mass units.

#895 Declined
  1. MattT

This relates to discussion on the mailing list, and needs signoff from the people pinged in the PR.

No longer on hold -- I get the right answers.

Comments (16)

  1. Sam Geen

    Looks good. I'll try testing it on some data with both particles and gas tomorrow, but in principle it should work fine on both.

  2. MattT author

    @nickolas1 I've been trying to remember why this change was made, and I cannot -- do you think the new version, here, looks correct?

  3. nickolas1

    It was PR62 where this happened in the older version of yt-3- it looks like you backed out the mass unit change after emailing Romain. I think the mass unit was only used for particles then, while the gas mass of a cell was calculated using the density unit and length units? If this has changed in newer versions, maybe that could account for the different behavior.

  4. Sam Geen

    Hmm, I'm actually getting paranoid about this boxlength multiplication - particle positions seem to be scaled between 0 and boxlen already. I need to check the grid positions to see if the same is true. One other thing is that my version of the code calculates boxlen inside the code itself, which could cause problems, although it's strange that updating caused the units to stop working.

    1. nickolas1

      would you mind holding off on closing/denying this PR for a few days? I'd like to figure out what's up with my dataset with disagreeing density extrema, but I just got back from vacation. I should have time Weds/Thurs.

  5. MattT author

    Well, crap. Bitbucket updated this pull request, even though I didn't want it to.

    Did we decide that we didn't want this? I'm going to -- for now -- back out the changes and decline the PR.

  6. Sam Geen

    Sorry, I was going to look at this and had a bunch of deadlines sneak up on me. I have identical plotting routines working in Pymses and YT, so can compare them side-by-side. My recollection is that the density units are OK, but that pressure/temperaure might be bugged as I think pressure was still only in code units (and I want to be 100% sure that the length units are consistent).