Geocoded burst stack processor (cont)

[scottstanie]

Creating this PR mostly from Virginia's #18 . I used it last week for 3-4 areas, so it's pretty functional. I came across several things as I was processing, notably

• the dates in the dataframe/code were integers, but were trying to be filtered using the pandas .str.contains
• The dates passed to --exclude-dates were actually being included, so I refactored the filter function to allow both exclusions or inclusions (but fixed the logic so --exclude-dates does exclude)

I think I've addressed several of the comments made on #18, so I was just trying to help fix up the final changes others can more easily work on the same code, without merging multiple forked branches.

[scottstanie]

@LiangJYu I believe I implemented all the suggestions you had in comments here #18 . The only one I didn't do was moving all pruning calls into the data frame generation, since after profiling, the pandas filtering was actually very quick even with a few thousand bursts.

Also noting that I added a --bbox option so that bursts outside some lat/lon bounding box are skipped, and all runconfigs are changed to have the corresponding bbox (converted to UTM coordinates).

So if there aren't objections, I'll merge this in shortly.

[LiangJYu]

LGTM. My comments are all formatting related.

[vbrancat]

@scottstanie thanks for all the improvements :). I only have minor comments at this stage. I am planning to test the PR pretty soon and will let you know if anything comes up.

One "big observation" I have is the following. Since we have a burst map, I was wondering whether for the stack processor, we want to use the info in the burst map to populate the runconfig (e.g. epsg, geogrid). We are following this direction for CSLC processing. We will soon have a place holder in the schema for the burst map and extrapolate epsg and geogrid bbox for each burst from the burst map itself. What do you think? We should not address this point in this PR but it would be good to keep in mind.

[hfattahi]

Thanks @scottstanie for restoring this PR. I put in few minor comment. I think it will be useful for us to have an option to use the burst ID database. Maybe it is not bad to allow the current behavior if the burst database is not provided.

Another point is do we want to generate the geocoded bursts only over the common ID among all dates? I think if we force processing over only common busrst IDs among all dates, then we could potentially exclude a lot of busrsts to process. An alternative would be to loop over the zip files and process each burst that statisfies the input constraints (Burst ID list, or bbox). This way we may end up with different number of coregsitered SLCs for different IDs.

[vbrancat]

Another nit-picking consideration. Shall we consider renaming the s1_geo_stack.py function? In the main folder, we opted for explicitly spell the word geocode. May be something like s1_geocoded_cslc_stack_processor? It might be too long but something like that would be helpful.

[scottstanie]

Another nit-picking consideration. Shall we consider renaming the s1_geo_stack.py function? In the main folder, we opted for explicitly spell the word geocode. May be something like s1_geocoded_cslc_stack_processor? It might be too long but something like that would be helpful.

@vbrancat What about just s1_geocode_stack to match the 3-part names of s1_geocode_slc and s1_geocode_metadata?

[vbrancat]

@scottstanie For next time, we do implicitly require that, for each PR, "at least" two people grant approvals. This is fine for now, but please, make sure to follow this rule for your next PR.