add fix for timing model

[siyuan-chen]

This PR addresses #161 by changing the sign of the residuals - new_res. In addition, there is a need to sort the TOAs, as the libstempo pulsar object and the enterprise pulsar object have different sorting for the TOAs.

[codecov-commenter]

Codecov Report

Merging #162 (71ad252) into master (01a1cd7) will increase coverage by 1.76%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #162      +/-   ##
==========================================
+ Coverage   34.38%   36.15%   +1.76%     
==========================================
  Files          19       19              
  Lines        3711     3820     +109     
==========================================
+ Hits         1276     1381     +105     
- Misses       2435     2439       +4     

Impacted Files	Coverage Δ
enterprise_extensions/timing.py	28.57% <0.00%> (-1.43%)	⬇️
enterprise_extensions/models.py	41.20% <0.00%> (-0.83%)	⬇️
enterprise_extensions/sampler.py	39.45% <0.00%> (+3.92%)	⬆️
enterprise_extensions/empirical_distr.py	73.86% <0.00%> (+14.60%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01a1cd7...71ad252. Read the comment docs.

[Hazboun6]

@ark0015 Do these changes match the changes you've made on your branch? Would you mind taking a look through these changes?

[ark0015]

This looks like the corrections I have, except I also sort the residuals so that they are both sorted the same. So mine looks like:
return residuals[isort] - new_res[isort].

[Hazboun6]

So @ark0015 do you think that @siyuan-chen changes should also sort the residuals?

[ark0015]

I believe that both need to be sorted. Here is an example comparison where I changed the parallax (by 10 sigma) and compute the tm_delay both with unsorted residuals (top panel) and sorted residuals (bottom panel). Only when sorting both residuals and new_res do you get the signature error when the parallax is incorrect.

[siyuan-chen]

@ark0015 Just to check: When you don't sort neither residuals nor new_res, do you get the correct parallax signature ?
I added the sorting to the residuals[isort] in the pull request. This should be an additional safety.

I think that when you pass the residuals through an enterprise pulsar object, then there is no need to sort the residuals. See: https://github.com/nanograv/enterprise/blob/master/enterprise/pulsar.py#L199
But, since new_res comes from libstempo, there is no sorting applied. Which then mismatches the enterprise pulsar sorting.

[ark0015]

@siyuan-chen It looks like you might be right that when neither of them are sorted, they reproduce the parallax signal. I think that because tm_delay uses the libstempo version of the pulsar object, which to my knowledge does not sort the residuals, we should sort them both. The differences between the unsorted and sorted is small, but it may be worth getting ahead of here. Attached is the comparison of both unsorted (top), unsorted residuals (top middle), both sorted (bottom middle), and the difference between both sorted and both unsorted (bottom).

[siyuan-chen]

Thanks, Andrew. Yeah, let's go with sorting both. The PR already has the code changed to sort both, so it just needs to be accepted. @Hazboun6