-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow for an artificial horizon when generating an *Observer sky map. #25
base: master
Are you sure you want to change the base?
Conversation
PR Summary
Overall, this PR provides an important update to sky observation functionalities, allowing for an elevation parameter to be considered, and extends test coverage, making our codebase more robust. |
Platform dependent masking fraction? Oh, boy, that's going to be fun to figure out... |
LGTM! FWIW, I would be happy to merge w/o a perfect test as the masking fraction issue is very strange. I've just updated the codecov |
I'd like to find some way of making it less flakey -- whether it's by using isclose like you mentioned the other day, or by finding a value that reduces the noise/jitter between platforms (i.e., a high elevation where there is a lower fraction of masked pixels). I'll hopefully have time to find some kind of resolution for this later this week. |
I seem to have run into a case of -ffast-math optimizations differing by platform, resulting in the tests returning different values on my ARM64 laptop, vs the x86_64 runners. Increasing the elevation reduces the effective number of boundary pixels, reducing the chance of the optimizations resulting in a variable number of pixels being masked by platform.
Looks like that's intentional, https://github.com/telegraphic/pygdsm/blob/master/.github/workflows/python-app.yml#L37 |
Looks like CodeCov is out of action again, but the tests were successful this time |
This adds a kwarg to BaseObserver.generate() to allow for the horizon to be set at above 0 degrees, allowing for only a fraction of the visible sky to be sampled.
Additionally, I have added tests to ensure that only a fraction of the sky is visible, and that the rad/deg convention used by pyephem for the lon/lat values is also respected for the elevation keyword.