SlideRuleEarth / sliderule

Server and client framework for on-demand science data processing in the cloud
https://slideruleearth.io
Other
25 stars 11 forks source link

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

Closed elidwa closed 3 weeks ago

elidwa commented 4 weeks ago

/ 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 commented 4 weeks ago

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 commented 4 weeks ago

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 commented 3 weeks ago

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