Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Representation of particle mass versus density #83

Closed
jwise77 opened this issue Feb 23, 2021 · 5 comments
Closed

Representation of particle mass versus density #83

jwise77 opened this issue Feb 23, 2021 · 5 comments

Comments

@jwise77
Copy link
Contributor

jwise77 commented Feb 23, 2021

I'm opening up an issue to have a discussion on the particle masses. This was brought up in #49. The particles in the group has_mass will be included in the gravity solve. Currently, the mass field is defined to be density. This is how Enzo handled the particle masses to ease their mass deposition into grids for the gravity solve.

Do we keep this convention from Enzo? Or do we rename the field to particle_mass_density or the like? Then we could have some derived field that contains their masses. I could be convinced either way.

@matthewturk
Copy link
Contributor

This would somewhat reduce the complexity on the yt side.

@stefanarridge
Copy link
Contributor

Yes I agree this is something that needs to be sorted out, although @jobordner already created an issue for this last year (#54). I have been planning to look into this more in the next couple of weeks.

@gregbryan
Copy link
Contributor

I think we should break with Enzo and define mass as mass. It will just simplify things and make the code easier to read and modify.

@stefanarridge
Copy link
Contributor

I agree @gregbryan. At the moment, as far as I can tell, the particle 'masses' are kept the same when blocks are refined or coarsened, which is not consistent with treating 'mass' as density. I believe that is the reason for these lines of code in src/Enzo/enzo_EnzoMethodPmDeposit.cpp

// Scale mass by volume if particle value is mass instead of density

      int level = block->level();

      // Required for Cosmology ("mass" is mass)
      // Not for Collapse ("mass" is density)
      if (cosmology) {
        dens *= std::pow(2.0,rank*level);
      }

This PmDeposit method would be simplified a lot by simply depositing the particle mass to the grid before dividing by the cell volume. One thing that I would like to clear up is the treatment of comoving vs proper quantities. There are lots of statements all over the code looking something like:

if (cosmology){
x *= cosmo_a;
}

I have generally found it quite confusing to keep track of whether quantities are comoving or proper. In this case, do we need to divide by the comoving or proper cell volume. I also discussed this in #85 with respect to the gravity time step. It might need to be the subject of a separate Issue.

@mabruzzo
Copy link
Contributor

Resolved by PR #89

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants