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

Atl03Reader, Atl03BathyReader polyregion function may have an incorrect check for full extent found #412

Closed
elidwa opened this issue Jun 10, 2024 · 3 comments

Comments

@elidwa
Copy link
Contributor

elidwa commented Jun 10, 2024

/* If Coordinate Is NOT In Polygon */
if(!inclusion && segment_ph_cnt[segment] != 0)
{
break; // full extent found!
}

If segment is not in the polygon (inclusion == FALSE) there is no need to check if segments ph cnt is zero or not. Code should only check for inclusion. Before changing JP needs to verify if there was a reason for this check, if there was, please document in code.

@elidwa
Copy link
Contributor Author

elidwa commented Jun 10, 2024

extrasegments
correctsegments

Added two screen shots of plots. In one Atl03Viewer has extra segments outside of the bbox. That was due to the wrong check for full extent found. The second screenshot has only segments contained in the bbox. The extra segments outside of bbox have 0 photon count.

Notice that at the time Atl03Viewer implementation used the same polyregion function as Atl03Reader and BathyReader. This is no longer the case. Atl03Viewer has simplified polyregion implementation.

@jpswinski
Copy link
Member

jpswinski commented Jun 11, 2024

The photon count for the segment being zero is a special case in the ATL03 data and those segments must be ignored. There are cases when a zero-count segment has a bogus latitude and longitude - therefore using those segments to determine inclusion inside a polygon can prematurely treat the track as having entered or exited the polygon.

Therefore the code is correct as is. But we should add a comment to the code explaining what's above to make it clearer in the future.

@elidwa
Copy link
Contributor Author

elidwa commented Jun 12, 2024

Comment was added on branch: allrowsfix, pull request: #414

@elidwa elidwa closed this as completed Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants