search

21cmfast / 21cmFAST

Official repository for 21cmFAST: a code for generating fast simulations of the cosmological 21cm signal
MIT License
58 stars 37 forks source link

21cm fast heating #285

Closed debanjan-cosmo closed 1 year ago

debanjan-cosmo commented 2 years ago

21cmFAST with CMB and Ly-\alpha Heating

This branch includes CMB and Lyman-\alpha heating into the 21cmFAST code

comp_diff_quantities

delta21_k

delta21_z

steven-murray commented 2 years ago

One more thing -- please add the new parameters to the docstring in inputs.py, and add the journal references in those docstrings.

debanjan-cosmo commented 2 years ago

One more thing -- please add the new parameters to the docstring in inputs.py, and add the journal references in those docstrings.

Thank you very much @debanjan-cosmo. The code all looks good to me, and thank you for the plots, that really helps. Three things:

  1. Can @qyx268 do a little review of the physics here?
  2. @debanjan-cosmo Can you report the time difference running with your two new flag options versus without?
  3. Can you please remove the file src/py21cmfast/_version.py and add it to the .gitignore?

Thank you again!

Hi @steven-murray,

  1. It will be really nice. Thanks @qyx268
  2. The time difference with the new flags and without is about 7 seconds using a single processor. I have used all the previously mentioned parameters for the runs.
  3. I have now removed src/py21cmfast/_version.py and added it to .gitignore.
  4. I have added the new parameters to the docstring in inputs.py.

Thanks a lot!

Best, @debanjan-cosmo

BradGreig commented 2 years ago

Hi @debanjan-cosmo, just a couple of things in addition to @qyx268's detailed comments.

Can you comment why the CMB heating appears to do nothing to the signal. Unlike that shown in Reis+ (2021). Also, am I correct that their equation 8 is incorrect by a factor of 2/3 (compared to your 2022 publication). Is this because the Meiksin (2022) term is lower in amplitude? (I haven't checked whether it is lower amplitude than your 2022 publication).

Also, the impact of the Lyman-alpha heating appears different to that from Reis+ (2021). In your implementation the impact of Lyman-alpha heating occurs at late redshifts. However, in their publication it differs much earlier.

Secondly, you don't appear to update the coupling coefficients (x_\alpha and x_rad) with these additional terms. Is there a reason for this? I'm assuming for the latter its the usual approximation on e^-{\tau}. Note that 21cmFAST can explicitly compute this relation now.

debanjan-cosmo commented 2 years ago

HI @BradGreig ,

Thanks for the comments and questions. I'll answer those one by one.

(i) Our equation 6 and eqn 8 of Reis+ 2021 are same, both of which are taken from T. Venumadhav + Rev. D 98, 103513 (2018). The only thing, that is possibly wrong with Reis + 2021, is their definition of x_e. The denominator with the dx_e/dz term is (1+x_e), whereas it is (1+x_e+f_He) with rest of the heating terms (X-ray, CMB, Ly-alpha). We think that they have used a mixed definition. I have explained this in a previous reply in this thread.

(ii) Meiksin + 2022 argued that T. Venumadhav + 2018 have missed a crucial term, which makes the CMB heating more efficient that it really is. Meiksin + 2022 have corrected that mistake and the resulting effect is so slow that CMB heating practically has no effect. We, in our 2022 paper, have used the CMB heating from T. Venumadhav + 2018 (or Reis +2021). In this code, @andreimesinger has told me to use the Meiksin 2022 CMB heating.

The CMB heating term is computed using the last term in eq. 4 of Meiksin 2022 https://arxiv.org/pdf/2105.14516.pdf. You have to express P_CMB as (3/4)(TCMB/T*)A_10. Then you have to divide the whole term by (3/2) n_tot k_B, and have to apply cosmic-time to redshift conversion. This is what I have done to calculate the CMB heating inside the code.

(iii) Reis + 2021 have implemented the multiple scattering of the Ly-alpha photons, which enhances the heating effect. We have not included this effect. Also, there are possible errors in their equations, which I have discussed with @andreimesinger . The enhancement due to multiple scattering (or the errors?) may be causing this to happen early than ours.

Multiple scattering is something that we are currently working on, and hopefully will be able to implement in the 21cmFAST soon.

(iv) x_\alpha, I think, depends on the total Ly-alpha flux. So, should not be changed. I can include the x_rad calculation. However, we have found that the effect is negligible. In order to reduce the iterative calculation of x_rad, I can use the T_s-previous. It'll be easier. I'll do it.

Thanks again.

BradGreig commented 2 years ago

Hi @debanjan-cosmo thanks for the responses. It appears the inconsistencies in the equations was in the arxiv version of the Reis paper. They are consistent in the final version.

One other suggestion I would make is changing USE_Lya_HEATING -> USE_LYA_HEATING. While I don't think there is an actual consistent naming convention yet, given all other options are (mostly) capitalised, it probably makes sense to capitalise it.

Otherwise it all looks good to me.

steven-murray commented 1 year ago

@debanjan-cosmo this seems almost ready to be merged -- can you have another look at it?

debanjan-cosmo commented 1 year ago

Sure! I’ll. Thanks.

On Fri, 28 Oct 2022 at 2:41 Steven Murray @.***> wrote:

@debanjan-cosmo https://github.com/debanjan-cosmo this seems almost ready to be merged -- can you have another look at it?

— Reply to this email directly, view it on GitHub https://github.com/21cmfast/21cmFAST/pull/285#issuecomment-1294066155, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUR6I2BUIYSS2CEL7I6EEXLWFLVZVANCNFSM5XCHRDGA . You are receiving this because you were mentioned.Message ID: @.***>

-- DEBANJAN SARKAR

----------------------------------------------------------- Department of Physics Ben-Gurion University of the Negev Beer-Sheva, Israel Mob: +972-53-8540546 Email: @. @.

andreimesinger commented 1 year ago

Hi @debanjan-cosmo , could we prioritize this PR? We are planning on launching a large database of runs and it would be good to have this merged so that Lya heating is included. Many thanks.

debanjan-cosmo commented 1 year ago

Hi, I have solved a few conflicts. Also updated my branch. I am not sure how to make another PR. Could you please help?

steven-murray commented 1 year ago

Hi @debanjan-cosmo you don't need to make a new PR I don't think. This one is good.

andreimesinger commented 1 year ago

@debanjan-cosmo just looking at the PS vs z evolution, I am surprised to see the absence of the usual three peaks in the signal corresponding to Lya coupling, Xray heating, and EoR. In particular, the Lya coupling peak seems to be missing... Do you understand why? Also the curves are very noisy for some reason. Perhaps you are using the PS calculated from the lightcone instead of the co-eval boxes, which could explain the noisiness and the smearing out of the Lya coupling peak?

In any case, it would be good for the paper to make these with higher resolution, at least DIM = 512, HII_DIM=128, BOX_LEN=250 or so

andreimesinger commented 1 year ago

@debanjan-cosmo said it is only 7 extra seconds on a single core. And as I was saying it isn’t so much a “feature” as a heating channel… There is no flag to turn off compton heating for example…

On 06.12.2022., at 14:39, Steven Murray @.***> wrote:

@steven-murray commented on this pull request.

In src/py21cmfast/inputs.py https://github.com/21cmfast/21cmFAST/pull/285#discussion_r1040986064:

@@ -634,6 +638,8 @@ class FlagOptions(StructWithDefaults): defaults = { "USE_HALO_FIELD": False, "USE_MINI_HALOS": False,

  • "USE_CMB_HEATING": True,
  • "USE_Lya_HEATING": True, Do they add extra computation time? Our approach in the past has been to make each new feature that adds computation time off by default, and can be turned on to make things more realistic. By the way, I'm still working on making a PR that will make it easy to switch between different configs like "fast", "realistic", etc.

— Reply to this email directly, view it on GitHub https://github.com/21cmfast/21cmFAST/pull/285#discussion_r1040986064, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADH2UAW4DT2XXYTXDFJABGTWL462ZANCNFSM5XCHRDGA. You are receiving this because you were mentioned.

steven-murray commented 1 year ago

OK, sounds reasonable. We can leave as True by default.

debanjan-cosmo commented 1 year ago

@debanjan-cosmo just looking at the PS vs z evolution, I am surprised to see the absence of the usual three peaks in the signal corresponding to Lya coupling, Xray heating, and EoR. In particular, the Lya coupling peak seems to be missing... Do you understand why? Also the curves are very noisy for some reason. Perhaps you are using the PS calculated from the lightcone instead of the co-eval boxes, which could explain the noisiness and the smearing out of the Lya coupling peak?

In any case, it would be good for the paper to make these with higher resolution, at least DIM = 512, HII_DIM=128, BOX_LEN=250 or so

Hi @andreimesinger, yes, I am using lightcone boxes for calculating PS. I have run some high-res simulations. I am analysing the results. I'll add the results here.

debanjan-cosmo commented 1 year ago

@andreimesinger @steven-murray I have checked the code. There was a minor bug. Actually, there are 2/3 versions of the code that I am modifying. By mistake, I had included a flag before reading the Ly-alpha heating table in this version of the code, and I uploaded the same to GitHub. It was causing a core dump. I have double-checked everything and run several tests. I hope this version will not show any errors. Please let me know if you still see any errors.

debanjan-cosmo commented 1 year ago

Hi @andreimesinger @steven-murray, I think I have now updated the files. They should work now. Could you please run a test and let me know? Thanks.

steven-murray commented 1 year ago

Hi @debanjan-cosmo, it it is still running into some segmentation fault errors. You'll also need to do a git pull on the branch since I merged master into your branch.

debanjan-cosmo commented 1 year ago

Hi @debanjan-cosmo, it it is still running into some segmentation fault errors. You'll also need to do a git pull on the branch since I merged master into your branch.

Hi @steven-murray, can you please tell me exactly where it is showing segmentation fault? I looked at the log but was unable to understand it. I am running the tests (given in the 'tests' directory) locally on a mac (intel, macOS Ventura 13.0.1), and did not find any such errors. The tests running fine. Is it possible to share the exact script that you are using to run these tests? It will be really very helpful. Thanks.

debanjan-cosmo commented 1 year ago

Hi @steven-murray @andreimesinger @qyx268, the test 'test_cli.py' shows a segmentation fault in the test_spin() function. I found that it is showing the error exactly after the debugger label "LOG_SUPER_DEBUG("looping over box...");". I am not sure why the error is occurring. I have allocated all the variables properly and parallelizations also seem to be okay. I have run the code for a few cases, with different flags on/off. It seems to be working fine. I have tried both pdb and gdb, but could not understand the exact reason for this error. Could you please help me with this?

steven-murray commented 1 year ago

Hi @debanjan-cosmo it is still segfaulting -- looks like it's failing in the test_cli::test_spin function. What I recommend doing is running::

PYTHONMALLOC=malloc valgrind --tool=memcheck --track-origins=yes --leak-check=full --suppressions=devel/valgrind-suppress-all-but-c.supp pytest -v tests/test_cli.py::test_spin > valgrind.out 2>&1

See here for more info.

debanjan-cosmo commented 1 year ago

Hi @steven-murray, thanks. I'll try it and let you know.

andreimesinger commented 1 year ago

thanks.  could you also remember to attach the new PS plots?On 13 Dec 2022, at 00:40, Debanjan Sarkar @.***> wrote: Hi @steven-murray, thanks. I'll try it and let you know.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

debanjan-cosmo commented 1 year ago

Sure, I’ll do it by tomorrow or the day after. I have to decorate the plots a bit before uploading.

On Tue, 13 Dec 2022 at 1:43 andreimesinger @.***> wrote:

thanks. could you also remember to attach the new PS plots?On 13 Dec 2022, at 00:40, Debanjan Sarkar @.***> wrote: Hi @steven-murray, thanks. I'll try it and let you know.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/21cmfast/21cmFAST/pull/285#issuecomment-1347528456, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUR6I2DP3BTL6DF4XORPO43WM62DNANCNFSM5XCHRDGA . You are receiving this because you were mentioned.Message ID: @.***>

debanjan-cosmo commented 1 year ago

Here are some of the plots with "DIM":600, "HII_DIM": 200, "BOX_LEN": 300, and log10(LX) = log10(LX_Mini) = 39.

  1. EOS parameters: T_21_eos_z p_21_eos_k p_21_eos_kz

  2. OPT parameters: T_21_opt_z p_21_opt_k p_21_opt_kz

Let me know if you want to see any specific plots and I'll make those. Thanks.

andreimesinger commented 1 year ago

thanks debanjan. are you computing these from the lightcones or the coevals?  i still am surprised there are no three peaks.  if you look at figures 10 and 12 here: https://arxiv.org/pdf/2110.13919.pdfthere are definitely three peaksOn 13 Dec 2022, at 21:52, Debanjan Sarkar @.***> wrote: Here are some of the plots with "DIM":600, "HII_DIM": 200, "BOX_LEN": 300, and log10(LX) = log10(LX_Mini) = 39.

EOS parameters:

OPT parameters:

Let me know if you want to see any specific plots and I'll make those. Thanks.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

debanjan-cosmo commented 1 year ago

Hi @andreimesinger, I am using lightcone boxes. I think that is due to the choice of parameter LX = LX_Mini =39. Now I have made the following plots for LX = LX_Mini = 40.5. Now we can see the three peaks. I have used nchunks=25, so my line-of-sight (z) bins are sparse.

  1. EOS: p_21_eos_kz

  2. OPT p_21_opt_kz

andreimesinger commented 1 year ago

ok great. thanks.  i have no further questions :-)On 13 Dec 2022, at 21:52, Debanjan Sarkar @.***> wrote: Here are some of the plots with "DIM":600, "HII_DIM": 200, "BOX_LEN": 300, and log10(LX) = log10(LX_Mini) = 39.

EOS parameters:

OPT parameters:

Let me know if you want to see any specific plots and I'll make those. Thanks.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

debanjan-cosmo commented 1 year ago

Hi @steven-murray @andreimesinger, it seems that MacOS13 (Ventura) has dropped the support for Valgrind. I have been trying to install it but failed. I do not have any other computer where I can test things locally. Can anyone do the check this time for me? In the meantime, I'll arrange other means to test this.

debanjan-cosmo commented 1 year ago

ok great. thanks.  i have no further questions :-)On 13 Dec 2022, at 21:52, Debanjan Sarkar @.> wrote: Here are some of the plots with "DIM":600, "HII_DIM": 200, "BOX_LEN": 300, and log10(LX) = log10(LX_Mini) = 39. EOS parameters: OPT parameters: Let me know if you want to see any specific plots and I'll make those. Thanks. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.>

Thanks @andreimesinger :)

andreimesinger commented 1 year ago

Any luck on getting this through?

On 13.12.2022., at 22:25, Debanjan Sarkar @.***> wrote:

Hi @steven-murray https://github.com/steven-murray @andreimesinger https://github.com/andreimesinger, it seems that MacOS13 (Ventura) has dropped the support for Valgrind. I have been trying to install it but failed. I do not have any other computer where I can test things locally. Can anyone do the check this time for me? In the meantime, I'll arrange other means to test this.

— Reply to this email directly, view it on GitHub https://github.com/21cmfast/21cmFAST/pull/285#issuecomment-1349737266, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADH2UAVCTBVCXMB2RRS22ILWNDSS5ANCNFSM5XCHRDGA. You are receiving this because you were mentioned.

steven-murray commented 1 year ago

@debanjan-cosmo I can try to run valgrind over it, but I am pressed for time. I'll prioritize it as much as I'm able.

steven-murray commented 1 year ago

@debanjan-cosmo valgrind returned the following:

image

Looks like kappa_10 is doing an invalid double read. Seems fairly likely that the culprit is that TK is too large here:

image

We should definitely be handling this more gracefully, but perhaps you want to think about why TK can get this large now with your updates.

debanjan-cosmo commented 1 year ago

Hi @andreimesinger. Sorry, I was a bit busy with the interviews and job applications. I managed to install the virtual box on my mac and ran 21cmfast in it. I was trying out the different options to set up the debugging environment.

Hi @steven-murray, thanks a lot for running Valgrind and sending me this report. I'll definitely look into this problem with Tk. I'll work on it this weekend. Thanks again for this.

andreimesinger commented 1 year ago

@debanjan, any progress on this??

On 05.12.2022, at 03:29, Steven Murray @.***> wrote:

@debanjan-cosmo https://github.com/debanjan-cosmo valgrind returned the following:

https://user-images.githubusercontent.com/1272030/209402499-4f834db5-cfeb-4808-8eb4-e3d00a66d681.png Looks like kappa_10 is doing an invalid double read. Seems fairly likely that the culprit is that TK is too large here:

https://user-images.githubusercontent.com/1272030/209402834-bb2b7a93-29cb-4428-8442-6384166e4600.png We should definitely be handling this more gracefully, but perhaps you want to think about why TK can get this large now with your updates.

— Reply to this email directly, view it on GitHub https://github.com/21cmfast/21cmFAST/pull/285#issuecomment-1364319378, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADH2UAW65ZFAWCL3LATFIITWOYDUPANCNFSM5XCHRDGA. You are receiving this because you were mentioned.

debanjan-cosmo commented 1 year ago

@debanjan, any progress on this?? … On 05.12.2022, at 03:29, Steven Murray @.***> wrote: @debanjan-cosmo https://github.com/debanjan-cosmo valgrind returned the following: https://user-images.githubusercontent.com/1272030/209402499-4f834db5-cfeb-4808-8eb4-e3d00a66d681.png Looks like kappa_10 is doing an invalid double read. Seems fairly likely that the culprit is that TK is too large here: https://user-images.githubusercontent.com/1272030/209402834-bb2b7a93-29cb-4428-8442-6384166e4600.png We should definitely be handling this more gracefully, but perhaps you want to think about why TK can get this large now with your updates. — Reply to this email directly, view it on GitHub <#285 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADH2UAW65ZFAWCL3LATFIITWOYDUPANCNFSM5XCHRDGA. You are receiving this because you were mentioned.

@andreimesinger I have almost managed to solve it. I had several job interviews over the last two weeks. So did not get enough time to sit with the code. I now know where the problem is. I have applied a solution to bypass the problem. Now it is showing no error while running tests. I need 1-2 more days to refine the solution. I have some more interviews in the upcoming week. So I request you to kindly give me that time.

andreimesinger commented 1 year ago

ok thanks and good luck in the interviews!On 9 Jan 2023, at 02:01, Debanjan Sarkar @.***> wrote:

@debanjan, any progress on this?? … On 05.12.2022, at 03:29, Steven Murray @.***> wrote: @debanjan-cosmo https://github.com/debanjan-cosmo valgrind returned the following: https://user-images.githubusercontent.com/1272030/209402499-4f834db5-cfeb-4808-8eb4-e3d00a66d681.png Looks like kappa_10 is doing an invalid double read. Seems fairly likely that the culprit is that TK is too large here: https://user-images.githubusercontent.com/1272030/209402834-bb2b7a93-29cb-4428-8442-6384166e4600.png We should definitely be handling this more gracefully, but perhaps you want to think about why TK can get this large now with your updates. — Reply to this email directly, view it on GitHub <#285 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADH2UAW65ZFAWCL3LATFIITWOYDUPANCNFSM5XCHRDGA. You are receiving this because you were mentioned.

@andreimesinger I have almost managed to solve it. I had several job interviews over the last two weeks. So did not get enough time to sit with the code. I now know where the problem is. I have applied a solution to bypass the problem. Now it is showing no error while running tests. I need 1-2 more days to refine the solution. I have some more interviews in the upcoming week. So I request you to kindly give me that time.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

andreimesinger commented 1 year ago

any updates on this debajan?

On 9. 1. 2023., at 00:57, Andrei Mesinger @.***> wrote:

ok thanks and good luck in the interviews!

On 9 Jan 2023, at 02:01, Debanjan Sarkar @.***> wrote:



@debanjan https://github.com/debanjan, any progress on this?? … <x-msg://56/#> On 05.12.2022, at 03:29, Steven Murray @.***> wrote: @debanjan-cosmo https://github.com/debanjan-cosmo https://github.com/debanjan-cosmo https://github.com/debanjan-cosmo valgrind returned the following: https://user-images.githubusercontent.com/1272030/209402499-4f834db5-cfeb-4808-8eb4-e3d00a66d681.png https://user-images.githubusercontent.com/1272030/209402499-4f834db5-cfeb-4808-8eb4-e3d00a66d681.png Looks like kappa_10 is doing an invalid double read. Seems fairly likely that the culprit is that TK is too large here: https://user-images.githubusercontent.com/1272030/209402834-bb2b7a93-29cb-4428-8442-6384166e4600.png https://user-images.githubusercontent.com/1272030/209402834-bb2b7a93-29cb-4428-8442-6384166e4600.png We should definitely be handling this more gracefully, but perhaps you want to think about why TK can get this large now with your updates. — Reply to this email directly, view it on GitHub <#285 (comment) https://github.com/21cmfast/21cmFAST/pull/285#issuecomment-1364319378>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADH2UAW65ZFAWCL3LATFIITWOYDUPANCNFSM5XCHRDGA https://github.com/notifications/unsubscribe-auth/ADH2UAW65ZFAWCL3LATFIITWOYDUPANCNFSM5XCHRDGA. You are receiving this because you were mentioned.

@andreimesinger https://github.com/andreimesinger I have almost managed to solve it. I had several job interviews over the last two weeks. So did not get enough time to sit with the code. I now know where the problem is. I have applied a solution to bypass the problem. Now it is showing no error while running tests. I need 1-2 more days to refine the solution. I have some more interviews in the upcoming week. So I request you to kindly give me that time.

— Reply to this email directly, view it on GitHub https://github.com/21cmfast/21cmFAST/pull/285#issuecomment-1374904130, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADH2UAVKH4WEB3EBYXJ2RS3WRMFIXANCNFSM5XCHRDGA. You are receiving this because you were mentioned.