Starlink / starlink

Starlink Software Collection
162 stars 53 forks source link

sla: Fix bug in eqeqx and revise two related test routines. #73

Closed MalcolmCurrie closed 3 years ago

MalcolmCurrie commented 3 years ago

The bug causes observed places (the direction of the incoming radiation from a celestial source) to be wrong by an amount that was zero at J2000.0 but grows steadily and at the present time has reached almost 0.1 arcseconds.

The story is as follows.

I doubt there are any applications out there where this bug has had any practical effect. The pollution has been smooth and gradual, and is at least an order of magnitude below the level important for telescope pointing; in any case, pointing models have probably been fudging it out. The most critical would probably be interferometer delays, and such applications really ought to be using modern post IAU 2000 CIO based algorithms.

timj commented 3 years ago

Was there a line-ending difference between the two files? The diff is showing the entire file even though there are clearly lines that are the same before and after.

MalcolmCurrie commented 3 years ago

Compared with the old sla_test.f, I did add a line feed after the END statement. Might there be a Microsoft vs. Linux difference of line feeds throughout, and dos2unix needs to be applied? I should check to see if there an option in meld (used via git diff) to show LF changes. All imeld showed me were the individual lines changed and the new (non-Starlink) subroutines.

If that's the issue, I can redo this it in a fresh branch.

timj commented 3 years ago

It won't need a new branch. You can force push when you've updated the commit (since this is all on a branch). I haven't got a local check out of the repo at the moment so can't quickly do it myself.

MalcolmCurrie commented 3 years ago

Indeed. I meant a local branch so that I could redo the selection of revised lines from sla_test.fto be sure I was using the correct file version, but it would seem that doing the picking and copying in meld fixed the LFs. Thus only the converted eqeqx.f needed to be added to the second forced push.

grahambell commented 3 years ago

The changes were merged into the main branch.