FRBs / pygedm

Python bindings for YMW16, NE2001 and YT2020 electron density models
https://pygedm.readthedocs.io/
42 stars 17 forks source link

Check tau_sc tests are correct #18

Closed telegraphic closed 3 years ago

telegraphic commented 3 years ago

In PR #17 @InesPM added a frequency parameter to the methods, so that the scattering timescales are computed at a given frequency.

I have added some simple tests in test_tau_sc.py. Need to confirm the output values are indeed as expected.

    dm, tau_sc_1GHz = pygedm.dm_to_dist(0, 0, 1000, method='ymw16', nu=1.0)
    dm, tau_sc_100MHz = pygedm.dm_to_dist(0, 0, 1000, method='ymw16', nu=0.1)
    assert np.isclose(tau_sc_1GHz.value, 0.31681767)
    assert np.isclose(tau_sc_100MHz.value, 3168.17671061)
    assert np.isclose((0.1/1.0)**(-4) * tau_sc_1GHz.value, tau_sc_100MHz.value)

    dm, tau_sc_1GHz = pygedm.dm_to_dist(0, 0, 1000, method='ne2001', nu=1.0)
    dm, tau_sc_100MHz = pygedm.dm_to_dist(0, 0, 1000, method='ne2001', nu=0.1)
    assert np.isclose(tau_sc_1GHz.value, 198.57881596)
    assert np.isclose(tau_sc_100MHz.value, 4988074.33385041)    

@InesPM could you please take a look and confirm if these values are as expected? (Or do you have other tests that would be more appropriate)?

InesPM commented 3 years ago

Hi @telegraphic , I have checked the NE2001 model and they use a scattering index of -4.4 instead of -4 (as expected in a turbulent medium). For coherence between YMW16 and NE2001, in ymw16_wrapper.py tau_sc should be defined as follows:

tau_sc = r['tau_sc'] * nu ** -4.4 * u.s
telegraphic commented 3 years ago

Hi @InesPM

I just had a re-read of the YMW16 and it says:

In the absence of reliable data about the distribution and strength of scattering regions in the Galaxy, 
we adopt a simple approach to this, using the relation obtained by Krishnakumar et al. (2015) 
for the DM dependence of observed τsc values 
(in units of seconds and scaled to 1 GHz assuming τsc ∝ ν−4.0):

So I'm tempted to keep -4.0 for YMW16, as that is what the authors intended. Thoughts?

(We should however note in the pygedm documentation that we use -4.4 for NE2001 and -4.0 for YMW16).

InesPM commented 3 years ago

Hi @telegraphic, in that case I think it makes sense to keep -4.0 for YMW16.

telegraphic commented 3 years ago

Ok, thanks @InesPM, I'm going to close this, and we can re-open if/when errors are found.