search

sambit-giri / tools21cm

MIT License
22 stars 19 forks source link

Temperature correction in _dt_full buggy #8

Closed NGillet closed 4 years ago

NGillet commented 4 years ago

in the function _dt_full :

garrelt commented 4 years ago

Thanks Nicolas for pointing this out. I will correct the parameter error. Indeed when correct is .false. the spin temperature should just be used as is, so Ts_new=Ts. I suspect the routine was never used without correction.

However, I do not get the point about the equation Ts_new = Ts-xi T_HII / (1.-xi) and ( Ts-xi T_HII ) / (1.-xi) will do exactly the same thing. The expressions ab/c, (ab)/c and a*(b/c) all have the same outcome.The parentheses are simply not necessary. Did you mean something else?

Thanks, Garrelt

garrelt commented 4 years ago

Sorry, my mistake I did not look carefully and you are entirely correct. The idea is that the code returns the average temperature in the cell . If we then assume that when a cell has an ionization fraction of xi, a fraction xi is fully ionized at a temperature T_HII and the rest of the cell is fully neutral at a temperature T_HI then T_HI=( - xi*T_HII)/(1-xi) so with parentheses. I have now corrected this in my copy and sent Sambit a pull request for this change.

sambit-giri commented 4 years ago

@NGillet Do the above comments resolve your concerns?

NGillet commented 4 years ago

Yes, it solves the issue. Thank you.

garrelt commented 4 years ago

Dear Nicolas,

Thanks for confirming this. We had some internal discussion on whether this correction should be there in tools21cm at all. It’s not very sophisticated and having it as part of the calculation of the brightness temperature sort of hides it for the user.

Furthermore, Hannah Ross actually developed a better diff. brightness function which includes the Ly-a coupling. For this function the kinetic temperature is just an input so it’s up to the user to make any corrections to it. She never used the erroneous function you discovered for any of her work.

Would it be ok to upgrade to this more sophisticated dTb function or is the current function is any way useful for your research?

Thanks, Garrelt

On 22 Sep 2020, at 09:29, Nicolas notifications@github.com wrote:



Yes, it solves the issue. Thank you.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/sambit-giri/tools21cm/issues/8#issuecomment-696556054, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAGRFG4UF34ZFO2URQSBSO3SHBG6TANCNFSM4QUXTFLA.

NGillet commented 4 years ago

You can update it. I am not using it directly, I mostly compare to my direct simulations output for double-checking.