Fix a subtle los - to - enu bug

[jlmaurer]

Description

• There is a sign error in the LOS calculation using inclination angle and heading
• There is a direction ambiguity in the LOS; you can be either pointing towards the sensor from the ground or vice-versa

How Has This Been Tested?

• verified using end-member cases that this is correct.

• added unit tests to verify the different directions

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• I have added an explanation of what changes do and why I'd like you to include them.
• I have written new tests for your core changes, as applicable.
• I have successfully ran tests with changes locally.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

[jlmaurer]

@bbuzz31 can you check this and verify whether you think it's correct?

[jlmaurer]

@dbekaert I was looking at the heading angles in S1 and ALOS-2, and I'm wondering if the problem is that the heading angle reported is not pixel-to-sensor CW from north. If that's the case, that would explain the "bug" which would then actually be a bug in our description of the heading angle. Do you know how heading is typically defined or where it might be defined?

[bbuzzanga]

@jlmaurer there is a post on the ISCE forum describing the convention. Unfortunately the forum doesn't properly exist anymore. You can try and find the post here if you want: isce-framework/isce2#342

I had a preliminary unsuccessful look, but am on too slow of a wifi connection to dig

[jlmaurer]

Yes, the existing code is based on that post, but I'm not sure what convention is assumed for heading. Here's my thought experiment: inclination 90 degrees, heading 0, 90, 180, 270. What should the unit vector be in each case? The current code gives [-1, 0, 0] for 90 and [1, 0, 0] for 270, which I think is backwards.

[dbekaert]

Suggest to take a definition consistent with nisar.

…

On Tue, Nov 9, 2021 at 6:17 PM Jeremy Maurer ***@***.***> wrote: Yes, the existing code is based on that post, but I'm not sure what convention is assumed for heading. Here's my thought experiment: inclination 90 degrees, heading 0, 90, 180, 270. What should the unit vector be in each case? The current code gives [-1, 0, 0] for 90 and [1, 0, 0] for 270, which I think is backwards. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#329 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AESZPSLSLUZXQIP3XYPIIELULHI43ANCNFSM5HWNME4A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[jlmaurer]

Suggest to take a definition consistent with nisar.
…
On Tue, Nov 9, 2021 at 6:17 PM Jeremy Maurer @.***> wrote: Yes, the existing code is based on that post, but I'm not sure what convention is assumed for heading. Here's my thought experiment: inclination 90 degrees, heading 0, 90, 180, 270. What should the unit vector be in each case? The current code gives [-1, 0, 0] for 90 and [1, 0, 0] for 270, which I think is backwards. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#329 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AESZPSLSLUZXQIP3XYPIIELULHI43ANCNFSM5HWNME4A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

I just found this document that seems to suggest (Table 6-18, oswSpecRes) that the Sentinel-1 heading angle is CCW from N. If that’s the case with NISAR then we should leave the code as is and change the comment, with appropriate modification to the unit tests.

I can't find any documentation on how NISAR headings will be reported.

[jlmaurer]

@sssangha suggested that ISCE heading is counter-clockwise (CCW) from east, not CW. So I need to update the comment, not the code. Existing code should work fine.

[jlmaurer]

@sssangha can you add a reference to the relevant ISCE code if you know where it is?

[jlmaurer]

Link to relevant ISCE code:https://github.com/insarlab/MintPy/blob/ce7a27c63936b56bf2f870363a8c245ef2096df5/mintpy/utils/utils0.py#L381

[jlmaurer]

Closing as this is now addressed by #331