Merge newchem-cpp branch #264
Conversation
brittonsmith
force-pushed
the
newchem-cpp
branch
from
d11cdfb to
b0e23cd
Compare
|
@mabruzzo we seem to have some merger conflicts with main now. Are you able to resolve these? |
|
I’m not at my computer at this moment, but I will check shortly
…On Wed, Apr 9, 2025 at 11:52 AM Britton Smith ***@***.***> wrote:
@mabruzzo <https://github.com/mabruzzo> we seem to have some merger
conflicts with main now. Are you able to resolve these?
—
Reply to this email directly, view it on GitHub
<#264 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AG3EHBWVM4SHJ5ROL445A4D2YU63DAVCNFSM6AAAAABZH6TIOGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOOJQGE4DGOBYGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
*brittonsmith* left a comment (grackle-project/grackle#264)
<#264 (comment)>
@mabruzzo <https://github.com/mabruzzo> we seem to have some merger
conflicts with main now. Are you able to resolve these?
—
Reply to this email directly, view it on GitHub
<#264 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AG3EHBWVM4SHJ5ROL445A4D2YU63DAVCNFSM6AAAAABZH6TIOGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOOJQGE4DGOBYGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
That force push just happened because I accidentally pushed to this branch 2 seconds earlier (the force-push simply removed the new commit) |
src/clib/solve_rate_cool_g.F
Outdated
| endif | ||
|
|
||
| ! ignore metal chemistry/cooling below this metallicity | ||
| min_metallicity = 1.d-9 / z_solar |
There was a problem hiding this comment.
In solar units, this is a metallicity of just under 1e-7 Zsun. I suspect I've done this wrong and that the min metallicity is meant to be 1e-9 Zsun. This would be consistent with other places in the code where this is done by hand.
| ! HII, HeII, HeIII recombination heating | ||
|
|
||
| edot(i) = edot(i) - chunit * ( | ||
| & 13.6_DKIND*( k1(i) * HI(i,j,k) * de(i,j,k) | ||
| & - k2(i) * HII(i,j,k) * de(i,j,k) ) | ||
| & + 24.6_DKIND*( k3(i) * HeI(i,j,k) * de(i,j,k)/4._DKIND | ||
| & - k4(i) * HeII(i,j,k) * de(i,j,k)/4._DKIND ) | ||
| & + 79.0_DKIND*( k5(i) * HeII(i,j,k) * de(i,j,k)/4._DKIND | ||
| & - k6(i) * HeIII(i,j,k)* de(i,j,k)/4._DKIND ) | ||
| & ) | ||
|
|
There was a problem hiding this comment.
@gregbryan, @jwise77 I would really like to get your thoughts on this block of code here. With this, we are now adding a heating term proportional to the recombination rate minus the collisional ionization rate. This sort of cancels out the conventional recombination cooling term in cool1d_multi_g.F. I think the assumption here is that recombination photons are being reabsorbed. Does that sound right? The main result is a slightly higher (~20%) equilibrium temperature when a uv background is present. Does this seem like a reasonable addition?
The second thing to point out is that this term is not being applied in cool1d_mult, but in solve_rate_cool near to where chemical heating from H2 formation is being applied. This basically means that the cooling rate applied during solve_rate_cool is inconsistent with what one would derive by calling calculate_cooling_time. I assume this was done here as it relies on the k-rate values, which are not being passed into cool1d_multi. If this is a term worth keeping, do you think it's worth moving into cool1d_multi properly?
src/clib/calc_tdust_1d_g.F
Outdated
| real*8, parameter :: kgr1 = 4.0e-4_DKIND / 0.009387d0 | ||
| real*8, parameter :: kgr200 = 16.0_DKIND / 0.009387d0 |
There was a problem hiding this comment.
I concur that this normalization needs to be there. Specifically, equation 17 of Omukai (2000) gives kappa at Z = Z_local. However, just below that this paper seems define dust-to-gas ratio at Z_local as 0.934e-2 whereas the value used here is from Pollack (1994). As well, the T^2 proportionality used here is taken from Dopcke et al. (2011), who have a slightly different normalization (by about a factor of 2). This at least needs to be pointed out here.
|
@brittonsmith, I made a minor mistake. I was intending to open a new PR that merged in the recent changes from main (and bump the gold-standard-nccv3 to gold-standard-nccv4 to account for the change from PR #367), but I accidentally pushed to this branch. This isn't a significant change, so I'm inclined to live with it, but let me know if you want me to revert it |
1. remove some unused variables (that I accidently copied) 2. prevent some implicit casts
…or handling statements will be removed in the future.
…ulated dust temperature within solve_rate_cool. **NOT YET TESTED**
…dust_growth_and_destruction
…tal density in dust_growth_and_destruction
…o changed read() commands to exit() so that the code does not hang and waste compute time
…h new variables in chemistry_data. Added defauly values for these in set_default_chemistry_data. Moved SEC_PER_YEAR macro into grackle_defs.h
[newchem-cpp] Add dust_growth_and_destruction commit history
…InfoEntry This information will only be used after we transcribe `calc_grain_size_increment_1d` (at which point we will remove the corresponding constants from phys_constants.h and can entirely delete the dust_const.def file
…e readability (and add some comments)
I moved them from initialize_dust_yields.cpp where the comments were out of place to dust/grain_species_info.cpp
…seidel [newchem-cpp] transcribe `step_rate_g`
[newchem-cpp] Introduce the `GrainSpeciesInfo` type
[newchem-cpp] convert `solve_chemistry.{c->cpp}` and `calculate_*.{c->cpp}`
This pull request merges the newchem-cpp branch, which contains several major updates including:
For more information, see PR #177 and PR #143.