Skip to content
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

Lack of consistency between fwd and fwd_intermediate azimuth output #1163

Closed
stephenwinn16 opened this issue Oct 19, 2022 · 10 comments · Fixed by #1165
Closed

Lack of consistency between fwd and fwd_intermediate azimuth output #1163

stephenwinn16 opened this issue Oct 19, 2022 · 10 comments · Fixed by #1165
Labels
Milestone

Comments

@stephenwinn16
Copy link

Problem description

I've noticed an inconsistency between the azimuth output of fwd and fwd_intermediate. In the example below, I compute the lon/lat/azi when moving 1km from London with an initial forward azimuth of 45 degrees with the both methods. As expected from the documentation, fwd returns the backward azimuth whereas fwd_intermediate returns the forward azimuth of the terminus point. For consistency and to avoid confusion, I suggest fwd_intermediate also return the backward azimuth.

Code Sample

from pyproj import Geod, enums

wgs84 = Geod(ellps="WGS84")

# Inputs 
london = (-0.118092, 51.509865) #lon/lat
dist = 1e3  # m
bearing = 45  # deg
flags = enums.GeodIntermediateFlag(0).AZIS_KEEP  # get fwd_intermediate to return azimuths

# fwd
lon, lat, azi = wgs84.fwd(*london, bearing, dist)

# fwd_intermediate (return first and last point)
res = wgs84.fwd_intermediate(*london, bearing, 2, dist, flags=flags, terminus_idx=0, initial_idx=0)

print("lon, lat, azimuth from fwd             :", lon, lat, azi)
print("lon, lat, azimuth from fwd_intermediate:", res.lons[-1], res.lats[-1], res.azis[-1])

which outputs:

lon, lat, azimuth from fwd             : -0.10790547005142687 51.51622011204643 -134.99202649526126
lon, lat, azimuth from fwd_intermediate: -0.10790547005142687 51.51622011204643 45.007973504738736

Environment Information

pyproj info:
pyproj: 3.4.0
PROJ: 9.1.0

Installation method

Installed via pip

@snowman2
Copy link
Member

cc: @idanmiara

@snowman2
Copy link
Member

Related #841

@idanmiara
Copy link
Contributor

Hi,
As we don't want to break backwards compatibility, I've added this option as a non-default argument.
Hope it helps :)

@stephenwinn16
Copy link
Author

Hi @idanmiara,
Thanks for the changes. I understand the argument for backward compatibility. However, wouldn't it make more sense for the default behaviour of 'fwd' and 'fwd_intermediate' to be the same? You could have 'return_back_azimuth' is True by default and a note in the next release warning prior user.

@idanmiara
Copy link
Contributor

idanmiara commented Oct 25, 2022

Hi @stephenwinn16,
I really don't mind to go either way or have any strong opinion.
I guess it should be @snowman2 's call.

@idanmiara
Copy link
Contributor

Hi @stephenwinn16, @snowman2
I've added return_back_azimuth to fwd() as well.
I would have made the default to return forward azimuth for both, but it doesn't make sense to change the default behavior of fwd so I'm not sure if we would prefer to preserve backwards compatibility with fwd_intermediate or to make the default value of return_back_azimuth=True for the sake of compatibility with fwd.
WDYT ?

@snowman2
Copy link
Member

I see this comment in the code:

           # back azimuth needs to be flipped 180 degrees
           # to match what PROJ geod utility produces.

This makes me think that the default return should be the back azimuth for consistency with PROJ. Thoughts?

@idanmiara
Copy link
Contributor

idanmiara commented Oct 25, 2022

I see this comment in the code:

           # back azimuth needs to be flipped 180 degrees
           # to match what PROJ geod utility produces.

This makes me think that the default return should be the back azimuth for consistency with PROJ. Thoughts?

makes sense, but I guess that the comment should be fixed:
by default, forward azimuth needs to be flipped 180 degrees to match the (back azimuth) output of PROJ geod utilities.

@idanmiara
Copy link
Contributor

idanmiara commented Oct 25, 2022

Hi @idanmiara, Thanks for the changes. I understand the argument for backward compatibility. However, wouldn't it make more sense for the default behaviour of 'fwd' and 'fwd_intermediate' to be the same? You could have 'return_back_azimuth' is True by default and a note in the next release warning prior user.

@snowman2 agrees with you, so I've changed the default.
I added this parameter to fwd, inv, fwd_intermediate, inv_intermediate

@stephenwinn16
Copy link
Author

This makes me think that the default return should be the back azimuth for consistency with PROJ. Thoughts?

Agreed.

@snowman2 agrees with you, so I've changed the default.

Great, this should make everything consistent. Thank you both!

@snowman2 snowman2 added this to the 3.5.0 milestone Nov 4, 2022
snowman2 added a commit that referenced this issue Dec 19, 2022
ENH: Add `return_back_azimuth: bool` to allow compatibility between the azimuth output of the following functions (issue #1163):
    `fwd` and `fwd_intermediate`, `inv` and `inv_intermediate`,
    Note: BREAKING CHANGE for the default value `return_back_azimuth=True` in the functions `fwd_intermediate` and `inv_intermediate`
    to mach the default value in `fwd` and `inv`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants