Skip to content

Wrong azimuth angle #6

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

Open
dinhnhat0401 opened this issue Mar 21, 2013 · 12 comments
Open

Wrong azimuth angle #6

dinhnhat0401 opened this issue Mar 21, 2013 · 12 comments
Assignees

Comments

@dinhnhat0401
Copy link

Sorry but i find that the sun azimuth must plus 180 degree so:
function getAzimuth(H, phi, dec) {
return atan(sin(H), cos(H) * sin(phi) - tan(dec) * cos(phi));
}

must be:
function getAzimuth(H, phi, dec) {
return PI + atan(sin(H), cos(H) * sin(phi) - tan(dec) * cos(phi));
}

@mourner
Copy link
Owner

mourner commented Mar 21, 2013

You're right, I used south-based azimuth whereas north-based azimuth is usually used for sun position...

@ghost ghost assigned mourner Mar 21, 2013
@florianmski
Copy link

Thank you ! I'm currently writing an Android app and I've ported this awesome tool to java and I didn't understand where was my mistake concerning the sun azimuth :)

xqjibz pushed a commit to xqjibz/suncalc that referenced this issue Oct 28, 2014
@xqjibz xqjibz mentioned this issue Oct 28, 2014
@xqjibz
Copy link

xqjibz commented Oct 28, 2014

I created PR #35 to address this with tests.

@ghost
Copy link

ghost commented Nov 10, 2014

just noticed this bug myself, thanks for the great work.

The pull request seems to fix it for me, will be double checking in the next days.

@spencerthayer
Copy link

Will fix this be merged into the main branch soon?

@mourner
Copy link
Owner

mourner commented Nov 19, 2014

Probably, but I'd suggest just adding a PI instead of waiting for it to be merged.

@warpling
Copy link

Is there a reason #35 hasn't been merged?

@mourner
Copy link
Owner

mourner commented Jun 26, 2015

Yeah, hesitating to break backwards compatibility, leading to even more confusion :) I'll deal with this eventually, just didn't get my hands on it yet.

@warpling
Copy link

Makes sense! Crazy how many forks are downstream also…

@ghost
Copy link

ghost commented Jun 27, 2015

If breaking backwards compatibility is a concern than it might be good to add a new interface method which would give the correct result and leave the old one for those who were doing their own workarounds.

@xqjibz
Copy link

xqjibz commented Jul 1, 2015

I did not consider backwards functionality when I did the PR, that's my bad, I simply updated my library here and ran with it. @mourner if you have another solution, I'd be happy to work something up, otherwise no harm no foul on the PR.

@mourner mourner added the bug label Oct 27, 2015
@mourner mourner added feature and removed bug labels Nov 11, 2015
@ultrafez
Copy link

According to semver, the way to avoid breaking users existing apps is to release a new version with a major version number change (i.e. release as 2.0.0). This way, the vast majority of users won't pull it into their project automatically.

@mourner mourner added the sun label Oct 11, 2018
zedxxx added a commit to sasgis/sas.planet.src that referenced this issue Jan 22, 2025
    - Wrong azimuth angle: mourner/suncalc#6
    - Nadir shifted from past to future: mourner/suncalc#125
    - Coefficient is incorrect in solarTransitJ() function: mourner/suncalc#178
zedxxx added a commit to zedxxx/suncalc-pas that referenced this issue Apr 17, 2025
    - Wrong azimuth angle: mourner/suncalc#6
    - Nadir shifted from past to future: mourner/suncalc#125
    - Coefficient is incorrect in solarTransitJ() function: mourner/suncalc#178
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants