Tf/baseline geometry - Part 1 & Part 2

[tfuhrmann]

First PR related to the implementation of baseline and geometry information into PyRate. The new functionality reads additional datasets from GAMMA interferogram stacks, i.e.:

• additional metadata from the slc/mli.par files,
• information on the spatial baseline vector of each interferogam as given in the *base.par files,
• the GAMMA lookup table file which is used to relate DEM rows/cols (and their respective lon/lat values) to radar-coded rows and cols (i.e. the azimuth/range coordinate system). Note that the binary lookup table file is transformed into numpy arrays for azimuth and range in PyRate.

After all relevant data is read per-pixel look angle, incidence angle and azimuth angle maps are calculated and saved as tif files in the out directory. The calculation of per-pixel perpendicular baseline values for each interferogram has also been implemented.

I've added some new files/additional parameters to the test data set, and also needed to add code to the PyRate tests/*.py-files to get the test suite up and running again. All tests are now running successfully using pytest (except for some of the known Legacy test failures in Gadi) and are also passed in Travis. Furthermore the compression algorithm used in write_output_geotiff needed to be changed from 'packbits' to 'LZW' to make all tests pass (might be related to the dodgy test data set and could potentially be changed back once the new S1 test data set is ready. Note that additional tests for the new data files and functionalities need to be added. Also, the implementation of the functionality is not finished (e.g. I want to add code to use the "initial baseline" in case the "precision baseline" is not available in the base.par files). I have raised this PR anyway now, since I wanted to get up-to-date with the latest release.

[codecov-commenter]

Codecov Report

Merging #302 into develop will decrease coverage by 0.00%. The diff coverage is 81.42%.

@@             Coverage Diff             @@
##           develop     #302      +/-   ##
===========================================
- Coverage    83.22%   83.22%   -0.01%     
===========================================
  Files           26       27       +1     
  Lines         3470     3845     +375     
  Branches       544      588      +44     
===========================================
+ Hits          2888     3200     +312     
- Misses         486      522      +36     
- Partials        96      123      +27     

Impacted Files	Coverage Δ
pyrate/core/gdal_python.py	85.33% <ø> (+1.12%)	:arrow_up:
pyrate/default_parameters.py	100.00% <ø> (ø)
pyrate/prepifg.py	50.74% <ø> (ø)
pyrate/core/gamma.py	80.35% <68.42%> (-12.43%)	:arrow_down:
pyrate/configuration.py	90.60% <75.00%> (-1.57%)	:arrow_down:
pyrate/core/config.py	92.82% <76.19%> (-1.60%)	:arrow_down:
pyrate/core/shared.py	92.42% <79.54%> (-0.73%)	:arrow_down:
pyrate/core/dem_error.py	90.84% <90.84%> (ø)
pyrate/constants.py	100.00% <100.00%> (ø)
pyrate/core/ifgconstants.py	100.00% <100.00%> (ø)
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d378a5f...89b3d36. Read the comment docs.

[mcgarth]

I think it's worth us having a discussion on when these new elements are computed. My thinking is that baseline, incidence and azimuth images are done during prepifg. They are not a 'correction'. They are tools used in correction(s) in the correct step. Furthermore, when we implement reprojection (to pseudo-vertical or -horizontal) I think it makes sense to do that in prepifg also, so we would need the azimuth and incidence images in that step. Clearly we need a dem_error.py module that is called in the correct step. But I am not sure the current contents of dem_error.py is best housed there. Open to hearing other peoples views though @tfuhrmann @adeane-ga @chandra2ga @basaks

[tfuhrmann]

I think it's worth us having a discussion on when these new elements are computed. My thinking is that baseline, incidence and azimuth images are done during prepifg. They are not a 'correction'. They are tools used in correction(s) in the correct step. Furthermore, when we implement reprojection (to pseudo-vertical or -horizontal) I think it makes sense to do that in prepifg also, so we would need the azimuth and incidence images in that step. Clearly we need a dem_error.py module that is called in the correct step. But I am not sure the current contents of dem_error.py is best housed there. Open to hearing other peoples views though @tfuhrmann @adeane-ga @chandra2ga @basaks

Thanks Matt for all your comments. I'll work on those today along with fixing the tests which are currently not passed in Travis. On your comment: I agree that the tools currently implemented in dem_error.py are not the correction as such but only the geometry/baseline input which will be required to calculate the correction. So, I'm happy to move those parts to the end of prepifg. The only reason I've put it in the correct step was that all input can be calculated 'on-the-fly' and doesn't need to be saved to disc necessarily (currently only output for testing). But yes, if we are going to make use of look/incidence/azimuth angle files in the future and want to output them anyway, it makes sense to move these parts to the end of perpifg and then read the look_angle.tif to calculate the DEM error correction.

[mcgarth]

I have added commit 8b9ba31 to make the calculation of bperp files and geometry files optional, triggered by presence of basefilelist and ltfile parameters in the config file

Once tests are passing, I am happy for this PR to be merged in to develop as a work in progress. Feedback below can be incorporated in to the next PR.

I have tested with the unit test data. When basefilelist and ltfile are in the config file, I find that the baseline metadata (six entries) are added to headers in ifg and coh geotiff files from both conv2tif and prepifg. Even without basefilelist or ltfile, additional metadata from the slc/mli.par files is saved in to the ifg/coh geotiff headers. During correct step, look_angle.tif, incidence_angle.tif, azimuth_angle.tif and dem_error/*bperp.tif are generated. It looks like the first three contain angles in units of radians (would degrees be a better choice for quick human interpretation?). Some thought needs to go in to the metadata to be included in the headers of these files; currently they're labelled as DATA_TYPE=MULTILOOKED_IFG, which isn't accurate. A DATA_UNITS entry could also be useful.

The function pyrate.core.shared.read_lookup_table is specific to a Gamma-format flat-binary FCOMPLEX lookup table file. I therefore think it would be better housed in pyrate.core.gamma and executed during prepifg.

[tfuhrmann]

I have added commit 8b9ba31 to make the calculation of bperp files and geometry files optional, triggered by presence of basefilelist and ltfile parameters in the config file

Once tests are passing, I am happy for this PR to be merged in to develop as a work in progress. Feedback below can be incorporated in to the next PR.

I have tested with the unit test data. When basefilelist and ltfile are in the config file, I find that the baseline metadata (six entries) are added to headers in ifg and coh geotiff files from both conv2tif and prepifg. Even without basefilelist or ltfile, additional metadata from the slc/mli.par files is saved in to the ifg/coh geotiff headers. During correct step, look_angle.tif, incidence_angle.tif, azimuth_angle.tif and dem_error/*bperp.tif are generated. It looks like the first three contain angles in units of radians (would degrees be a better choice for quick human interpretation?). Some thought needs to go in to the metadata to be included in the headers of these files; currently they're labelled as DATA_TYPE=MULTILOOKED_IFG, which isn't accurate. A DATA_UNITS entry could also be useful.

The function pyrate.core.shared.read_lookup_table is specific to a Gamma-format flat-binary FCOMPLEX lookup table file. I therefore think it would be better housed in pyrate.core.gamma and executed during prepifg.

Thanks Matt. I've worked on the tests today and the Travis tests should now (hopefully) be all passed. I've also fixed the DATA_TYPE for the three angle tifs and the bperp tifs. I'll move function read_lookup_table from shared.py to gamma.py in the next PR as discussed. The functions to calculate the various angles for the PyRate-multi-looked outputs (i.e. get_lonlat_coords, get_radar_coords, calc_local_geometry and vincinv) could be hosted in a new python file geometry.py which is called by the end of prepifg.py. Would that make sense to you?

[mcgarth]

Thanks Matt. I've worked on the tests today and the Travis tests should now (hopefully) be all passed. I've also fixed the DATA_TYPE for the three angle tifs and the bperp tifs. I'll move function read_lookup_table from shared.py to gamma.py in the next PR as discussed. The functions to calculate the various angles for the PyRate-multi-looked outputs (i.e. get_lonlat_coords, get_radar_coords, calc_local_geometry and vincinv) could be hosted in a new python file geometry.py which is called by the end of prepifg.py. Would that make sense to you?

Yes, that makes sense, to have these functions together in a module geometry.py (back in the day there was a geodesy.py module!). Also, it would be logical to have a test module test_geometry.py for unit testing these functions. You might also consider whether the functions pyrate.core.refpixel.convert_pixel_value_to_geographic_coordinate and pyrate.core.refpixel.convert_geographic_coordinate_to_pixel_value should be re-homed in geometry.py, given the two functions you mention: get_lonlat_coords, get_radar_coords.