Handle polar positions in height calculation#842
Handle polar positions in height calculation#842jamesgrimmett wants to merge 1 commit intoskyfielders:masterfrom
Conversation
An investigation in support of #842.
|
That was a fun problem! It's always a useful exercise when a bug gets me to pull out a pencil and paper and work out the details of formulae that I had pasted into Skyfield in haste. In this case the formulae came from: https://celestrak.org/columns/v02n03/ Dr. Kelso's expression for Thank you for contributing a possible solution! But I wanted to see whether there was a way to solve the imprecision without creating a special case or needing two paths through the code: Happily, I found that inside of the It requires returning an additional value from Thanks so much for reporting this, and for showing one way the problem could be solved—it was very helpful to start from a working solution! I'll plan to release a new Skyfield version by the end of the month; maybe sooner. In the meantime, here's how to install the development version to test out the fix on your own box if you'd like: |
|
Excellent! Thanks for sharing your working and thought process, it's very useful to see. I also felt that my proposed solution was a little less than ideal, so I'm glad that you found a tidier solution. I have been referring to Dr. Kelso's columns quite a bit while learning, so I will keep this alternative solution in mind (it's also a good reminder that there is always a chance the existing formulae can be improved or reworked!). I'll rebase the branch for #839 to include this new work. Thanks! |

I found an issue with the calculation of heights for polar positions while working on #839. The source of the problem is that the formula used to calculate height (
height_au = R / cos(lat) - aC) is not valid whenlat=+/-90, resulting in incorrect elevation in the output, e.g.,I've added a check for polar positions during height calculations, and in those calculations use
height_au = abs(xyz_au[2]) - self.semi_minor_axis.auinstead, which I think resolves the issue;