Particle masses bugfix #89

stefanarridge · 2021-04-28T13:53:35Z

This changes EnzoMethodPmDeposit so that that all particles in the 'has_mass' group are taken into account when calculating the mass field, and the particles' mass attribute/constant is always interpreted as a quantity with dimensions of mass, rather than sometimes density. This fixes issue #83.

This may cause some issues with some of the Collapse tests, since those were written assuming that 'mass' is really density.

Update (1 March 2022):

I have made the following changes:

1 - The "has_mass" group of particle types is now called "is_gravitating".
2 - Particle types can have either an attribute or a constant called either "mass" or "density". The pm_deposit method handles the four separate cases. "mass" is treated as a quantity with dimensions of mass, and "density" is treated as a quantity with dimensions of density.

I have updated some of the parameter files to reflect these changes, mostly by replacing "mass" with "density" and "has_mass" with "is_gravitating" in appropriate places. Users will have to update their parameter files likewise. I have not changed the IsolatedGalaxy parameter files, since it would appear (based on input/IsolatedGalaxy/README) that the "mass" attribute was already being treated as a mass quantity. I have also left alone the parameter files in input/MergeStars since the functionality of these tests do not depend on whether "mass" is a mass or density quantity.

Regarding the issue concerning the depositing of the gas density field raised by @mabruzzo , I have undone the changes which I previously made to this part of the pm_deposit method. However, since I believe that the way this is done currently is incorrect, and were not fixed by the changes I made in any case, I leave this for a future PR.

Update (8 March 2022):

This PR is now ready for review. I am re-requesting a review from @jwise77 since there have been substantial changes since he previously approved it.

…articleDescr classes

…eposit

src/Enzo/enzo_EnzoMethodPmDeposit.cpp

DeovratPrasad

someone who deals with particles algo needs to look at the comment by Stefan in file enzo_EnzoMethodPmDeposit.cpp . I have also left a comment there flagging the issue.

stefanarridge · 2021-05-18T18:21:58Z

someone who deals with particles algo needs to look at the comment by Stefan in file enzo_EnzoMethodPmDeposit.cpp . I have also left a comment there flagging the issue.

Thanks for pointing that out. That comment was due to a misunderstanding on my part. I'll delete it now.

jwise77

Nearly all of these changes look good to me. See my two comments. #49 included the has_attribute method as is_attribute. I can approve once these instances are changed to is_attribute (probably easier to make changes in this PR rather than the #49 changes) and the conflicts are fixed. Thanks!

jwise77 · 2021-05-21T18:11:28Z

src/Cello/data_ParticleDescr.cpp

@@ -253,6 +253,24 @@ int ParticleDescr::attribute_index (int it, std::string attribute_name) const

 //----------------------------------------------------------------------

+bool ParticleDescr::has_attribute(int it, std::string attribute_name) const
+{
+  ASSERT1("ParticleDescr::is_attribute",


Mismatch, but I'd just go with is_attribute to be consistent with #49

I named has_attribute as such to maintain consistency with the has_constant method. I have now decided to name the methods 'is_attribute' and 'is_constant' instead.

src/Cello/data_ParticleDescr.cpp

bwoshea · 2021-05-30T19:36:27Z

@stefanarridge it looks like this PR has some conflicts, as well as some comments. Would mind fixing the conflicts, addressing the comments, and then letting the reviewers know it's ready for review? Thank you!

jwise77

All looks good! I only had a minor comment that isn't related to the density -> mass changes.

src/Enzo/enzo_EnzoMethodPmDeposit.cpp

mabruzzo

This looks great! I've got a couple of minor suggestions. I'm a little concerned that one of your changes might have broken gas density deposition in cosmological simulations (there is a comment highlighting that in enzo_EnzoMethodPmDeposit). But I could definitely be wrong.

Apologies for not providing these suggestions sooner. I wasn't originally intending to fully review this because I didn't really understand the Particle machinery (but James's presentation at the workshop made things a lot clearer).

src/Cello/data_ParticleDescr.cpp

src/Enzo/enzo_EnzoMethodPmDeposit.cpp

mabruzzo · 2021-06-03T13:48:04Z

src/Enzo/enzo_EnzoMethodPmDeposit.cpp

-    if (rank >= 1) hx *= cosmo_a;
-    if (rank >= 2) hy *= cosmo_a;
-    if (rank >= 3) hz *= cosmo_a;


It seems like that removing this is going to affect the way that the gas density gets deposited (which happens at the end of this function).

If this is true, then it's a little disturbing that none of our automated tests seem to detect this.

Okay, I'll look into this. I was under the impression that that both gas density and particle density were comoving quantities. I suppose if a physical length scale is required for depositing the gas density, then I can define two separate variables i.e hx_com and hy_phys, or something similar.

I was just trying to point out that this change was going to modify the deposititon of the gas density.

You could definitely be correct (I know basically nothing about the units of cosmological simulations). In that case the old implementation must have been wrong.

Could somebody familiar with the density units used for cosmological simulations in Enzo quickly clarify this for us? @jwise77 @bwoshea @gregbryan

Yes, I know what you mean. It would be good to understand which quantities need to be passed to dep_grid_cic (i.e., physical or comoving). I am also concerned that the value of dtf which is passed is not correct. Currently it is set to alpha_, which is a dimensionless parameter which is 0.5 by default, rather than a quantity with dimensions of time.

In Enzo, the particle "mass" is actually a comoving density, adjusted for the level that it is on, so that when it gets deposited to the GravitatingMassField (which is the comoving density field used to calculate the gravitational acceleration), there is no multiplication by a volume. Here, I'm not quite sure what should be done -- the mass of a particle is a constant, so if you wanted to compute a comoving density, then you would want to divide by the comoving volume of the cell. Normally dx is comoving, so a factor of the scale factor is not required, but I have to admit I have not really looked at what is going on in this routine, so best to take what I say with a grain of salt.

@gregbryan , the purpose of this PR is to redefine the "mass" attribute, so that it is always treated as mass, rather than (sometimes) as density. As far as I know, in a cosmological simulation, length scales and densities are all comoving quantities, which is why I deleted the lines which multiply hx,hy,hz by "cosmo_a", so that, when the mass is multiplied by 1/(hx * hy * hz), the output is a comoving density.

The issue which @mabruzzo pointed out, is that this will affect the deposition of gas density, which is done by calling the 'dep_grid_cic' function:

`
FORTRAN_NAME(dep_grid_cic)(de,de_gas,temp,

vx, vy, vz, &dtf, rfield, &rank, &hxf,&hyf,&hzf, &mx,&my,&mz, &gxi,&gyi,&gzi, &nxi,&nyi,&nzi, &i0,&i0,&i0, &nx,&ny,&nz, &i1,&i1,&i1);`

Should hxf, hyf, hzf be comoving or physical length scales?

I'm also surprised that dtf is set equal to 'alpha_'. Should it not be the same as the 'dt' used to drift the particles, i.e.:
const double dt = alpha_ * block->dt() / cosmo_a;

I took a look at dep_grid_cic, and I think that it's effectively just advancing the density over dt (i.e. the analogous operation to drifting the particles). I'm also pretty sure that the output density de_gas has the same units as the input de

As an aside, I'm pretty sure that a lot of these arguments that get passed have to do with complications that arise from the patch-based AMR used in the original Enzo.

If I'm remembering correctly, the fields used to store gas properties store values in comoving units for cosmological simulations, so hxf, hyf, and hzf should probably be comoving.

Furthermore, since the "density_total" field is the sum of the "density" field (after drifting) and the "density_particle" field, I suspect that "density_total", "density_particle" and "density_particle_accumulate" should all be comoving densities (in cosmological simulations).

I could be wrong, but this makes the most sense to me. The easiest way to check this would probably be to run some tests and compare results against Enzo.

Without doing a deep-dive back into this, it's unclear to me if you've made any changes on this front. Lmk if you have made changes, and I'll take another look

I'm also surprised that dtf is set equal to 'alpha_'. Should it not be the same as the 'dt' used to drift the particles, i.e.:
const double dt = alpha_ * block->dt() / cosmo_a;

I agree that it should be the same dt as the particle drift, but in Enzo, dt is actually zero for the PPM solver but not the RK2 solver (diff from 2011).

I looked at a very old version of Enzo from 2005, and I found this comment that using a non-zero dt creates an instability. I think we should change it to zero to be consistent with Enzo.

float dt = (DepositTime - Time)/a; dt = 0; /* the previous line is correct but creates an instability */

OK, I looked this over and I think removing the multiplication by cosmo_a is the right thing to do. As others have pointed out, hx,hy,hz should be comoving, so this is not necessary. Regarding alpha_, this is for the particles, and I think that is only set to zero for the baryons, so OK to keep as is?

…prints the density rather than inv_vol

…thodPmDeposit and added comment to improve readability

…with constant mass, rather than separately for each batch

…field is now the same as that used to advance the particles

…n branch. This is likely not correct but this was done for purposes of consistency and to ensure that this branch only changes the treatment of particle masses.

…ented successful compilation.

Merged with main

… simplified the code a little bit

jwise77

All looks good, but I will comment on the dtf = _alpha factor in a reply to John R's comment.

brittonsmith · 2022-05-09T16:24:22Z

Sorry for the late comments here. Firstly, I don't foresee any issues with the yt frontend. The existence or absence of the "is_gravitating" attribute makes for an easy check for whether the data was made before and after this change.

I apologize if this has already been discussed and if so I'm happy to leave it. Supporting both density and mass particles seems unnecessary. Is there any benefit to defining particle masses as densities beyond skipping a multiplication/division? It seems like we are making things more complicated for ourselves by continuing to support both options.

stefanarridge · 2022-05-09T17:30:25Z

I apologize if this has already been discussed and if so I'm happy to leave it. Supporting both density and mass particles seems unnecessary. Is there any benefit to defining particle masses as densities beyond skipping a multiplication/division? It seems like we are making things more complicated for ourselves by continuing to support both options.

There is in face little to no computational benefit in using densities rather than masses: in the "density" case, the densities have to be multiplied by 2^(rank * level) (rank is the number of dimensions, level is the refinement level), while in the "mass" case, the masses have to be multiplied by the inverse cell volume.

In principle, I agree with you, it would be better not to have the "density" option, but I am allowing for this option in order to make it easier for users to convert their old input parameter files or IC files. For example, if you look at the changes made to input/Cosmology/method_cosmology,incl, all that was required was to change the name of the "mass" constant to "density" (and change the name of the "has_mass" group to "is_gravitating"). The alternative would have been to change the value of the "mass" constant by multiplying it by the root level cell volume.

brittonsmith · 2022-05-10T12:22:09Z

In principle, I agree with you, it would be better not to have the "density" option, but I am allowing for this option in order to make it easier for users to convert their old input parameter files or IC files. For example, if you look at the changes made to input/Cosmology/method_cosmology,incl, all that was required was to change the name of the "mass" constant to "density" (and change the name of the "has_mass" group to "is_gravitating"). The alternative would have been to change the value of the "mass" constant by multiplying it by the root level cell volume.

I think this is an opportunity for us to rip off a relatively painless bandaid. Since the density option still requires a renormalization to the local grid cell size, this doesn't even work the way it does in enzo-dev, so this could be a potential point of confusion for analysis or if more code is ported from that codebase. My personal opinion is that leaving the density option in leaves the door open for bugs, but I would like to hear other opinions. @bwoshea, @jwise77, @gregbryan, @fearmayo, @peeples, @mabruzzo ?

mabruzzo · 2022-05-10T12:59:51Z

In principle, I agree with you, it would be better not to have the "density" option, but I am allowing for this option in order to make it easier for users to convert their old input parameter files or IC files. For example, if you look at the changes made to input/Cosmology/method_cosmology,incl, all that was required was to change the name of the "mass" constant to "density" (and change the name of the "has_mass" group to "is_gravitating"). The alternative would have been to change the value of the "mass" constant by multiplying it by the root level cell volume.

I think this is an opportunity for us to rip off a relatively painless bandaid. Since the density option still requires a renormalization to the local grid cell size, this doesn't even work the way it does in enzo-dev, so this could be a potential point of confusion for analysis or if more code is ported from that codebase. My personal opinion is that leaving the density option in leaves the door open for bugs, but I would like to hear other opinions. @bwoshea, @jwise77, @gregbryan, @fearmayo, @peeples, @mabruzzo ?

I don't much (any) experience using particles, but FWIW, I'm all for ripping of this band-aid. It doesn't seem like there's much downside

gregbryan · 2022-05-10T14:24:59Z

src/Enzo/enzo_EnzoMethodPmDeposit.cpp

-    if (rank >= 1) hx *= cosmo_a;
-    if (rank >= 2) hy *= cosmo_a;
-    if (rank >= 3) hz *= cosmo_a;


OK, I looked this over and I think removing the multiplication by cosmo_a is the right thing to do. As others have pointed out, hx,hy,hz should be comoving, so this is not necessary. Regarding alpha_, this is for the particles, and I think that is only set to zero for the baryons, so OK to keep as is?

gregbryan · 2022-05-10T14:28:10Z

src/Enzo/enzo_EnzoMethodPmDeposit.cpp

+
+    enzo_float hxf = hx * cosmo_a;
+    enzo_float hyf = hy * cosmo_a;
+    enzo_float hzf = hz * cosmo_a;


I think this cosmo_a factors should not be here, for the same reason discussed earlier.

gregbryan · 2022-05-10T14:28:33Z

src/Enzo/enzo_EnzoMethodPmDeposit.cpp

+
+    enzo_float hxf = hx * cosmo_a;
+    enzo_float hyf = hy * cosmo_a;
+    enzo_float hzf = hz * cosmo_a;
    enzo_float dtf = alpha_;


Definitely agree this should be alpha_ * dt.

gregbryan · 2022-05-10T14:35:42Z

In principle, I agree with you, it would be better not to have the "density" option, but I am allowing for this option in order to make it easier for users to convert their old input parameter files or IC files. For example, if you look at the changes made to input/Cosmology/method_cosmology,incl, all that was required was to change the name of the "mass" constant to "density" (and change the name of the "has_mass" group to "is_gravitating"). The alternative would have been to change the value of the "mass" constant by multiplying it by the root level cell volume.

I think this is an opportunity for us to rip off a relatively painless bandaid. Since the density option still requires a renormalization to the local grid cell size, this doesn't even work the way it does in enzo-dev, so this could be a potential point of confusion for analysis or if more code is ported from that codebase. My personal opinion is that leaving the density option in leaves the door open for bugs, but I would like to hear other opinions. @bwoshea, @jwise77, @gregbryan, @fearmayo, @peeples, @mabruzzo ?

I also think it may be simpler just to only allow mass. I always regretted the Enzo decision to make it density (the level of confusion added was not worth the small gains). I do know that you have already invested quite a bit of effort, so would completely understand punting on that for now.

I think the bigger issue is that we need a test problem to verify the choices made. For example, there is a discussion about using hx or hx * cosmo_a. I think we have arrived at the right thing, but there is only one answer that is right and it would be great to be able to show it. For cosmology, one could test the growth rate of the Zeldovich pancake or the power spectrum for a cosmo run. For non-cosmology, maybe just the growth timescale for the collapse problem?

stefanarridge · 2022-05-10T19:46:46Z

I also think it may be simpler just to only allow mass. I always regretted the Enzo decision to make it density (the level of confusion added was not worth the small gains). I do know that you have already invested quite a bit of effort, so would completely understand punting on that for now.

It won't be much work to get rid of the "density" option. However, users will have to update parameter files / IC files which set a value for particle "mass", although it should be as simple as just multiplying by the root level cell volume.

I think the bigger issue is that we need a test problem to verify the choices made. For example, there is a discussion about using hx or hx * cosmo_a. I think we have arrived at the right thing, but there is only one answer that is right and it would be great to be able to show it. For cosmology, one could test the growth rate of the Zeldovich pancake or the power spectrum for a cosmo run. For non-cosmology, maybe just the growth timescale for the collapse problem?

I have been considering making a separate PR which will address the "gas density deposition" issue, as well as a few other issues with the gravity method which I think need to be addressed. The issue I have is that I am not completely certain what the dep_grid_cic function is doing. Am I right in thinking that, in the special case where the "source" and "destination" fields have the same dimensions, are on the same refinement level, and are not offset, which is the case here, that it essentially does the following:

Loop over each active cell
Take the position corresponding to the center of the cell, and compute a new position which is this position plus "velocity times dt", where "velocity" is the value of the velocity field in this cell, and "dt" is the suitable timestep.
Use this new position to do CIC deposition of the "source density" (the value of "density" in this cell) onto the destination field (a new field called de_gas in this case, but may as well be the "density_total" field).

The way that I think about it is that it is essentially depositing mass in the same way as is done for the particles, only this time, the "particle positions" are the cell centers, the "particle velocities" are the values of the velocity field, and the "particle masses" are the cell masses (density times cell volume). If this is indeed how its done, it would be relatively straightforward to write this down in C++ code in `src/Enzo/enzo_EnzoMethodPmDeposit.cpp", which would simplify things a lot.

Obviously, how to test whether its "correct" is a separate question which I'm not sure how to answer.

…"Particle:mass_is_mass" parameter, which will nearly always have the value "dummy", but there is a small chance it will have a different value. The purpose of this parameter is to be a flag for the Enzo-E frontend of yt, indicating that particle masses should be treated as quantities with dimensions of mass, rather than quantities with dimensions of density. It is only important that this parameter exists, it does not matter what value it has.

…e a "density" attribute / constant.

…ave a "mass" constant rather than a "density" constant, and the value of the "mass" constant is the value of the previous "density" constant multiplied by the root level cell volume.

stefanarridge · 2022-05-12T15:25:06Z

Update: I have now made the following changes:

I have removed the "density" option for gravitating particles. That is, if a particle type is in the "is_gravitating" group, then it must have either a constant or an attribute called "mass".
I have updated input/Cosmology/method_cosmology.incl so that "dark" particles have a constant called "mass" rather than "density", but the value of this constant has changed to reflect that it is a quantity with dimensions of mass rather than dimensions of density. This value was computed by multiplying the value of the "density" constant by the root level cell volume. For a cosmological simulation, where the units are defined such that the volume of the domain is 1, this is equivalent to dividing by the number of root level cells (32^3 in this case), which are specified by the Mesh:root_size parameter.
the ".parameters" and ".libconfig" output files always contain a parameter in the Particle group called "mass_is_mass". This is intended to be used by the yt Enzo-E frontend, to indicate whether the data was produced by a version of Enzo-E which includes the changes introduced by this PR. I had to hack the code a bit to add in this parameter, and it doesn't work quite as intended, but there will always be a parameter called "mass_is_mass", which will almost certainly be a string with the value "dummy". The chances it isn't are 1 in 2^29... there is also a 1 in 2^31 chance that something weird might happen, and it might produce something nonsensical which yt won't be able to read in. If you're wondering what's going on, its because there is a int variable which doesn't get initialised properly, and so essentially it has a random value. If this value is outside the range 0-7, "mass_is_mass" will be "dummy"... if it's inside that range, something else will happen.

Again, I suggest this doesn't get merged in until the yt Enzo-E frontend is updated, which @brittonsmith is working on. I welcome comments and questions in the meantime.

fearmayo · 2022-05-17T16:50:04Z

Do we need the "mass_is_mass" attribute? I guess we use it for now and then silently end support for it in due course?

brittonsmith · 2022-05-18T15:39:19Z

Do we need the "mass_is_mass" attribute? I guess we use it for now and then silently end support for it in due course?

This is what I'm using on the yt side to detect whether the data was made before of after this change, which determines whether the density->mass conversion happens. The yt PR with the relevant changes can be found here.

@stefanarridge

Previously the drift time for the gas density was set to the value of `alpha` (taken from `Method:pm_deposit:alpha`). As @stefanarridge pointed out in PR enzo-project#89 this didn't make a lot of sense. I also ran into issues with introducing a Stable Jeans Wave regression test in PR enzo-project#186 that required me to set `Method:pm_deposit:alpha` to 0. For comparison, the drift time for the gravitating mass particles is set to `alpha * dt / cosmo_a`, in which: - `alpha` again comes from `Method:pm_deposit:alpha` - `dt` is the time-step during the current cycle - `cosmo_a` is the scale factor computed at `time + alpha * dt` (where `time` is the time at the start of the current cycle) To investigate this issue I checked the original code from enzo-dev in `Grid_DepositBaryons.C`. I concluded that enzo-dev sets the gas density's drift-timestep should get set in exactly the same way as the Particle's drift timestep. However, when using the PPM and Zeus Hydro-solvers, the drift timestep is set to 0 (this is consistent with a comment @johnwise made during his review of PR enzo-project#189). Since our 2 primary hydro-solvers in Enzo-E (Ppm and VL+CT) currently handle these source terms with the same temporal order as these solvers, we now force the gas-deposition drift timestep to be 0 in `EnzoMethodPmDeposit`. We will need to revisit this exact behavior when we eventually modify the VL+CT solver to handle self-gravity in a higher temporal-order manner.

@stefanarridge

…posit Previously the drift time for the gas density was set to the value of `alpha` (taken from `Method:pm_deposit:alpha`). As @stefanarridge pointed out in PR enzo-project#89 this didn't make a lot of sense. I also ran into issues with introducing a Stable Jeans Wave regression test in PR enzo-project#186 that required me to set `Method:pm_deposit:alpha` to 0. For comparison, the drift time for the gravitating mass particles is set to `alpha * dt / cosmo_a`, in which: - `alpha` again comes from `Method:pm_deposit:alpha` - `dt` is the time-step during the current cycle - `cosmo_a` is the scale factor computed at `time + alpha * dt` (where `time` is the time at the start of the current cycle) To investigate this issue I checked the original code from enzo-dev in `Grid_DepositBaryons.C`. I concluded that enzo-dev sets the gas density's drift-timestep should get set in exactly the same way as the Particle's drift timestep. However, when using the PPM and Zeus Hydro-solvers, the drift timestep is set to 0 (this is consistent with a comment @johnwise made during his review of PR enzo-project#189). Since our 2 primary hydro-solvers in Enzo-E (Ppm and VL+CT) currently handle these source terms with the same temporal order as these solvers, we now force the gas-deposition drift timestep to be 0 in `EnzoMethodPmDeposit`. We will need to revisit this exact behavior when we eventually modify the VL+CT solver to handle self-gravity in a higher temporal-order manner.

stefanarridge added 4 commits April 13, 2021 14:56

Adds has_attribute() and has_constant() methods to the Particle and P…

f47382f

…articleDescr classes

EnzoMethodPmDeposit treats particle mass as mass always

5efcce4

Changed/ removed comments in EnzoMethodPmDeposit

1ea6e9c

Changed 'divide by inv_vol' to 'multiply by inv_vol' in EnzoMethodPmD…

b9d7d0c

…eposit

mabruzzo reviewed Apr 30, 2021

View reviewed changes

src/Enzo/enzo_EnzoMethodPmDeposit.cpp Outdated Show resolved Hide resolved

Changed 'Edited' to '@author'

fdedf5e

DeovratPrasad reviewed May 18, 2021

View reviewed changes

Removes a comment from EnzoMethodPmDeposit

aad8a46

jwise77 reviewed May 21, 2021

View reviewed changes

Merged with main

a8c5e3d

stefanarridge requested review from DeovratPrasad, mabruzzo and jwise77 June 2, 2021 19:19

jwise77 approved these changes Jun 2, 2021

View reviewed changes

src/Enzo/enzo_EnzoMethodPmDeposit.cpp Outdated Show resolved Hide resolved

jwise77 mentioned this pull request Jun 2, 2021

More consistent checking for negative densities and masses in PM solver #116

Open

mabruzzo requested changes Jun 3, 2021

View reviewed changes

stefanarridge added 4 commits June 10, 2021 20:57

EnzoMethodPmDeposit reports negative densities for rank = 1,2,3, and …

c7ff73c

…prints the density rather than inv_vol

Fixed bug in ParticleDescr is_attribute and is_constant methods

3df4172

Moved lines of code which calculate the inverse cell volume in EnzoMe…

79963a5

…thodPmDeposit and added comment to improve readability

EnzoMethodPmDeposit allocates the pmass array once per particle type …

0fc15c0

…with constant mass, rather than separately for each batch

jwise77 mentioned this pull request Jun 14, 2021

Create Zel'dovich pancake test #123

Open

stefanarridge added 3 commits June 23, 2021 15:49

In EnzoMethodPmDeposit, the timestep used to advance the gas density …

39b80f0

…field is now the same as that used to advance the particles

Merge branch 'main' into particle_masses

cc7a53d

pm_deposit method treats the gas density field the same as in the mai…

af82abf

…n branch. This is likely not correct but this was done for purposes of consistency and to ensure that this branch only changes the treatment of particle masses.

fearmayo self-requested a review February 3, 2022 15:52

stefanarridge added 3 commits February 14, 2022 16:49

Fixed typo in src/Enzo/enzo_EnzoMethodPmDepositDeposit.cpp which prev…

694100e

…ented successful compilation.

Merge branch 'main' into particle_masses

af96ff2

Merged with main

Renamed ParticleDescr::has_constant to ParticleDescr::is_constant and…

616d3f0

… simplified the code a little bit

jwise77 approved these changes May 2, 2022

View reviewed changes

gregbryan reviewed May 10, 2022

View reviewed changes

stefanarridge force-pushed the particle_masses branch from 34d8923 to dc5bbd1 Compare May 12, 2022 00:15

stefanarridge force-pushed the particle_masses branch from dc5bbd1 to af6e42f Compare May 12, 2022 00:18

stefanarridge added 3 commits May 12, 2022 14:52

Updated "pm_deposit" method to remove the option for particles to hav…

06f4fe8

…e a "density" attribute / constant.

Edited input/Cosmology/method_cosmology.incl, so that the particles h…

2ff1cdf

…ave a "mass" constant rather than a "density" constant, and the value of the "mass" constant is the value of the previous "density" constant multiplied by the root level cell volume.

Updated documention.

41bd6e0

stefanarridge merged commit 2ee1799 into enzo-project:main May 26, 2022

stefanarridge mentioned this pull request Jun 8, 2022

Particle:mass_is_mass is no longer hard-coded #193

Merged

mabruzzo mentioned this pull request Jun 11, 2022

VL+CT support gravity source terms #186

Merged

stefanarridge mentioned this pull request Jun 13, 2022

Test of Collapse methods fails #207

Open

WillHicks96 mentioned this pull request Jun 14, 2022

Fixing particle mass in test_cosmo.incl #214

Merged

jwise77 mentioned this pull request Jun 14, 2022

[BUGFIX] Isolated galaxy particle attribute checks #218

Merged

stefanarridge mentioned this pull request Jun 21, 2022

Added instructions for how to install yt from source to "Getting started" #235

Merged

mabruzzo mentioned this pull request Jun 21, 2022

Ambiguous meaning of "mass" #54

Closed

WillHicks96 mentioned this pull request Jun 24, 2022

STARSS Star Formation and Feedback #198

Merged

mabruzzo mentioned this pull request Jun 27, 2022

Light refactor of EnzoMethodPmDeposit::compute and gas-deposition drift-time bugfix #239

Merged

mabruzzo mentioned this pull request Jul 11, 2022

Representation of particle mass versus density #83

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Particle masses bugfix #89

Particle masses bugfix #89

stefanarridge commented Apr 28, 2021 •

edited

Loading

DeovratPrasad left a comment

stefanarridge commented May 18, 2021

jwise77 left a comment

jwise77 May 21, 2021 •

edited

Loading

stefanarridge Jun 2, 2021

bwoshea commented May 30, 2021

jwise77 left a comment

mabruzzo left a comment

mabruzzo Jun 3, 2021

stefanarridge Jun 3, 2021

mabruzzo Jun 3, 2021

stefanarridge Jun 3, 2021

gregbryan Jun 3, 2021

stefanarridge Jun 4, 2021

mabruzzo Jul 13, 2021 •

edited

Loading

mabruzzo Apr 5, 2022 •

edited

Loading

jwise77 May 2, 2022

gregbryan May 10, 2022

jwise77 left a comment

brittonsmith commented May 9, 2022

stefanarridge commented May 9, 2022

brittonsmith commented May 10, 2022

mabruzzo commented May 10, 2022

gregbryan May 10, 2022

gregbryan May 10, 2022

gregbryan May 10, 2022

gregbryan commented May 10, 2022

stefanarridge commented May 10, 2022 •

edited

Loading

stefanarridge commented May 12, 2022

fearmayo commented May 17, 2022

brittonsmith commented May 18, 2022

Particle masses bugfix #89

Particle masses bugfix #89

Conversation

stefanarridge commented Apr 28, 2021 • edited Loading

DeovratPrasad left a comment

Choose a reason for hiding this comment

stefanarridge commented May 18, 2021

jwise77 left a comment

Choose a reason for hiding this comment

jwise77 May 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwoshea commented May 30, 2021

jwise77 left a comment

Choose a reason for hiding this comment

mabruzzo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mabruzzo Jul 13, 2021 • edited Loading

Choose a reason for hiding this comment

mabruzzo Apr 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwise77 left a comment

Choose a reason for hiding this comment

brittonsmith commented May 9, 2022

stefanarridge commented May 9, 2022

brittonsmith commented May 10, 2022

mabruzzo commented May 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregbryan commented May 10, 2022

stefanarridge commented May 10, 2022 • edited Loading

stefanarridge commented May 12, 2022

fearmayo commented May 17, 2022

brittonsmith commented May 18, 2022

stefanarridge commented Apr 28, 2021 •

edited

Loading

jwise77 May 21, 2021 •

edited

Loading

mabruzzo Jul 13, 2021 •

edited

Loading

mabruzzo Apr 5, 2022 •

edited

Loading

stefanarridge commented May 10, 2022 •

edited

Loading