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

WIP: azimuth component calculation of solid earth tide #105

Closed
wants to merge 32 commits into from

Conversation

seongsujeong
Copy link
Contributor

@seongsujeong seongsujeong commented Mar 13, 2023

This PR is to add azimuth component of the solid earth tide conversion, as well as integrating and finalizing the other LUT corrections that are implemented so far.

The conversion algorithm is based on ETAD ATBD available here.

Note that the 3D tidal displacement computation is performed in units of meters in the global ITRF. Therefore, the outcome needs conversion into slant-range and azimuth timing geometry of the correction grid. This conversion is performed by computing the zero Doppler radar times of the grid position 𝑋𝑔 including the tidal displacement Δ𝑋𝑆𝐸𝑇 and forming the difference to the grid nominal grid radar times.

This PR needs to be merged after the PRs below are merged. Until then, this PR will be labeled as work in progress.
#103 #102

Also note that the algorithm here calculates range component as well as azimuth component by its nature.

src/compass/utils/lut.py Outdated Show resolved Hide resolved
src/compass/utils/lut.py Outdated Show resolved Hide resolved
Comment on lines +174 to +177
rg_set_temp, az_set_temp = solid_earth_tides(burst,
lat[dec_slice, dec_slice],
lon[dec_slice, dec_slice],
height[dec_slice, dec_slice],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
rg_set_temp, az_set_temp = solid_earth_tides(burst,
lat[dec_slice, dec_slice],
lon[dec_slice, dec_slice],
height[dec_slice, dec_slice],
dec_slice = np.s_[::dec_factor, ::dec_factor]
rg_set_temp, az_set_temp = solid_earth_tides(burst,
lat[dec_slice],
lon[dec_slice],
height[dec_slice],

I think the slice only needs to be altered once.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested the original code, and it seems the slicing is happening on the first axis of the array only. In my test case (Rosamond, 10/16/2022, t064_135523_iw1), the radargrid rasters' shapes were (109,418), and dec_factor was 42.
dec_slice was as below:

dec_slice
slice(None, None, 42)

Below are the shapes of the decimated arrays:

lat.shape
(109, 418)
lat[dec_slice].shape
(3, 418)
lat[dec_slice, dec_slice].shape
(3, 10)

So it seems we need to provide the slice instance twice for the 2d arrays. @vbrancat Can you take a look at the information above and confirm that the shape of the decimated array makes sense?

Below is a toy code to show how the slicing works in 2D numpy array:

import numpy as np

arr_original = np.arange(150).reshape((10,15))
dec_slice = np.s_[::5]
print(arr_original.shape)
arr_slice_1 = arr_original[dec_slice]
print(arr_slice_1.shape)

arr_slice_2 = arr_original[dec_slice, dec_slice]
print(arr_slice_2.shape)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I have fixed this in the next commit.

src/compass/utils/lut.py Show resolved Hide resolved
src/compass/utils/lut.py Outdated Show resolved Hide resolved
@seongsujeong seongsujeong added the do-not-merge PR that should not be merged in the main branch label Mar 15, 2023
@vbrancat vbrancat removed the do-not-merge PR that should not be merged in the main branch label Mar 28, 2023
vbrancat and others added 15 commits March 31, 2023 13:38
* Add RAiDER to the requirements file

* Exposed troposphere to the CSLC interface

* Compute troposphere delay using weather model

* Save computed troposphere range LUT

* Add Docker spec file

* Format delay_type logic to avoid repetition

* reduce line of codes

* fix specifile
only forge - no default or .condarc channels

* correctly force forge w/o defauluts

* Correct zenith delay formula

* Remove unnecessary flags that forced conda forge channel

---------

Co-authored-by: Liang Yu <liangjyu@gmail.com>
* Mod on Dockerfile for cslc_s1 point release and few improvement

* Docker image size optimized

* Removing the commands in the header  comment

* Initial commit after branch-out

* minor fix; metadata writeout

* Update src/compass/utils/elevation_antenna_pattern.py

Co-authored-by: vbrancat <virginia.brancato@jpl.nasa.gov>

* Update src/compass/utils/elevation_antenna_pattern.py

Co-authored-by: vbrancat <virginia.brancato@jpl.nasa.gov>

* Comments on PR addressed; docstring for `apply_eap_correction` fixed

* comment clarification

* comment clarification

* addressing codacy issues

* Applying EAP correction (or skip) before `range_split_spectrum`

* addressing codacy issue

* metadata field name changed

* Update src/compass/s1_geocode_slc.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* Update src/compass/s1_geocode_slc.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* Update src/compass/s1_geocode_slc.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* Update src/compass/s1_geocode_slc.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* converting burst ID to `str`

* using EPSG from geogrid, not from input DEM

* Revert "Merge branch 'eap_correction_into_s1_geocode_slc'"

This reverts commit d2d5971, reversing
changes made to b5e9341.

* adding azimuth FM rate mismatch into lut.py

* docstring fixed; variable name changed

* docstring for file added; raise error when DEM is not valid

* Update src/compass/utils/lut.py

Co-authored-by: Heresh Fattahi <hersh.fattahi@gmail.com>

* Update src/compass/utils/lut.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* updates on local

* returning az_lut as `isce3.core.LUT2d()` code based on the suggestion from @LiangJYu

* docstring revised

* removing redundant renaming of the variables

* Dockerfile for beta release

* Specfile updated

* specifile updated

* tag on beta.Dockerfile updated

* specifile updated

* typo fix

* beta.Dockerfile updated

* updates on `s1_geocode_slc.py`

* but fix

* updates on beta.Dockerfile

* Dockerfile renamed

* untested writing az/rg correction LUT to hdf5
moved EAP to corrections group

* restructed HDF5
LUT data not working yet

* fix doppler return type
clean up debugs

* fix paths

* fix correction group organization

* camel to snake
fix correction spacing

* restore s1_burst_metadata
reorg doppler corrections

* PEP8 and syntax fixes

* fix correction grouping

* specfile updated

* Specfile updated

* s1-reader version updated

* Added `0.2` into `version.Tag`

* remove radar grid usage from geo grid
orbit item names all snake
correction reorg

* fix CSLC raster validation

* fix typo

Co-authored-by: vbrancat <virginia.brancato@jpl.nasa.gov>

* Update src/compass/utils/lut.py

Co-authored-by: vbrancat <virginia.brancato@jpl.nasa.gov>

* Update src/compass/utils/lut.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* docstring revised

* ref -> reference w/r/t _epoch
add spacing datasets
add metadata path comparision in validation

* fix burst border
_ID to _id
added more metadata to enable RadarGridProduct reconstruction

* Separate bistatic and azimuth FM rate

* change function return unpacking

* add all parameters needed to reconstruct burst object

* wrap value assignment with try+except
move helper to helpers up top for visibility

* burst DB now static
correct h5 write exception type

* fix burst.center type

* fix writing empty burst polynomials

* fix misformed poly1d items

* rename geocoding interpolator item

* move runconfig

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* fix product name

* add az fm rate mismatch into `h5_helpers.py`

* reverting s1_geocode_slc.py

* reverting `lut.py`

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* spelling error

* fix descriptions

* Version number changed

* add `validate_cslc.py` to `setup.cfg`

* Flexibility on Docker's entrypoint to run the validation script

* PEP8

* add entrypoint to `validate_cslc.py`

* fix on `setup.cfg`

* Update src/compass/utils/validate_cslc.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* Change version for s1-reader

* Removing default value for `dem_path`

* Update src/compass/utils/lut.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* release date updated for `0.1.3`

* Change base Docker image into Oracle Linux

* typo fix

* Write `NaN` into HDF5 instead of `None` for metadata (opera-adt#82)

* Mod on Dockerfile for cslc_s1 point release and few improvement

* Docker image size optimized

* Removing the commands in the header  comment

* Initial commit after branch-out

* minor fix; metadata writeout

* Update src/compass/utils/elevation_antenna_pattern.py

Co-authored-by: vbrancat <virginia.brancato@jpl.nasa.gov>

* Update src/compass/utils/elevation_antenna_pattern.py

Co-authored-by: vbrancat <virginia.brancato@jpl.nasa.gov>

* Comments on PR addressed; docstring for `apply_eap_correction` fixed

* comment clarification

* comment clarification

* addressing codacy issues

* Applying EAP correction (or skip) before `range_split_spectrum`

* addressing codacy issue

* metadata field name changed

* Update src/compass/s1_geocode_slc.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* Update src/compass/s1_geocode_slc.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* Update src/compass/s1_geocode_slc.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* Update src/compass/s1_geocode_slc.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* converting burst ID to `str`

* using EPSG from geogrid, not from input DEM

* Revert "Merge branch 'eap_correction_into_s1_geocode_slc'"

This reverts commit d2d5971, reversing
changes made to b5e9341.

* adding azimuth FM rate mismatch into lut.py

* docstring fixed; variable name changed

* docstring for file added; raise error when DEM is not valid

* Update src/compass/utils/lut.py

Co-authored-by: Heresh Fattahi <hersh.fattahi@gmail.com>

* Update src/compass/utils/lut.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* updates on local

* returning az_lut as `isce3.core.LUT2d()` code based on the suggestion from @LiangJYu

* docstring revised

* removing redundant renaming of the variables

* Dockerfile for beta release

* Specfile updated

* specifile updated

* tag on beta.Dockerfile updated

* specifile updated

* typo fix

* beta.Dockerfile updated

* updates on `s1_geocode_slc.py`

* but fix

* updates on beta.Dockerfile

* Dockerfile renamed

* untested writing az/rg correction LUT to hdf5
moved EAP to corrections group

* restructed HDF5
LUT data not working yet

* fix doppler return type
clean up debugs

* fix paths

* fix correction group organization

* camel to snake
fix correction spacing

* restore s1_burst_metadata
reorg doppler corrections

* PEP8 and syntax fixes

* fix correction grouping

* specfile updated

* Specfile updated

* s1-reader version updated

* Added `0.2` into `version.Tag`

* remove radar grid usage from geo grid
orbit item names all snake
correction reorg

* fix CSLC raster validation

* fix typo

Co-authored-by: vbrancat <virginia.brancato@jpl.nasa.gov>

* Update src/compass/utils/lut.py

Co-authored-by: vbrancat <virginia.brancato@jpl.nasa.gov>

* Update src/compass/utils/lut.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* docstring revised

* ref -> reference w/r/t _epoch
add spacing datasets
add metadata path comparision in validation

* fix burst border
_ID to _id
added more metadata to enable RadarGridProduct reconstruction

* Separate bistatic and azimuth FM rate

* change function return unpacking

* add all parameters needed to reconstruct burst object

* wrap value assignment with try+except
move helper to helpers up top for visibility

* burst DB now static
correct h5 write exception type

* fix burst.center type

* fix writing empty burst polynomials

* fix misformed poly1d items

* rename geocoding interpolator item

* move runconfig

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* fix product name

* add az fm rate mismatch into `h5_helpers.py`

* reverting s1_geocode_slc.py

* reverting `lut.py`

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* Update src/compass/utils/h5_helpers.py

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>

* spelling error

* fix descriptions

* Version number changed

* add `validate_cslc.py` to `setup.cfg`

* Flexibility on Docker's entrypoint to run the validation script

* PEP8

* add entrypoint to `validate_cslc.py`

* fix on `setup.cfg`

* Update src/compass/utils/validate_cslc.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* Change version for s1-reader

* Removing default value for `dem_path`

* Update src/compass/utils/lut.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* release date updated for `0.1.3`

* Change base Docker image into Oracle Linux

* typo fix

* write NaN instead of None

Co-authored-by: Seongsu Jeong <seongsu.jeong@jpl.nasa.gov>
Co-authored-by: vbrancat <virginia.brancato@jpl.nasa.gov>
Co-authored-by: Liang Yu <liangjyu@gmail.com>
Co-authored-by: Heresh Fattahi <hersh.fattahi@gmail.com>
Co-authored-by: Zhang Yunjun <yunjunzgeo@gmail.com>

* Apply corrections in geocodeSlc module (opera-adt#78)

* Add Boolean flag to acitivate/disactivate LUT corrections

* Feed LUT corrections to geocode SLC

* Save LUTs in CSLC product when computed, skip otherwise

* Remove trailing space

* Add cumulative LUT function

* Modify CSLC validation script (opera-adt#77)

* Throw error when percentage of pixels is above threshold

* Address comments on threshold and relative difference:

* Specify threshold in the print message. Suppress numpy warnings

* Use masked array; modify failed pixel percentage computation

* Use masked array to compute NaNs

* Correct division by reference pixels

* More verbose way to compute absolute difference

* compare raster per pixel
use masked arrays
assign thresholds to variables
report on both real and imag % fails

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* make common burst filtering optional in stack processor (opera-adt#60)

* make common burst filtering optional in stack processor

* also add gix for bad function name

* Update dependencies (opera-adt#88)

* Add pandas dependency

* add lmxl

* Include default values in runconfig written to HDF5 metadata (opera-adt#87)

* add default options to metadata runconfig str
* Update src/compass/utils/runconfig.py
Co-authored-by: Scott Staniewicz <scott.stanie@gmail.com>

* Unit test for IONEX functionalities (opera-adt#80)

* Unit test for IONEX functionalities
* Add ionex unit test data
* adopt pytest fixtures
* Allocate variables in conftest
* rewrite Docker command
* Test with pytest in circleci
docker - moved copy of source after miniconda install
* fix tec URL

* ionospheric correction - working version

* `ionex_file` added into `dynamic_ancillary_file_group` in runconfig

* Interface revision for iono / tropo corrections; test code removed

* `ionex_path` added

* shortcut to ionex path in cfg

* reverting the changes in parameters for calling `az_fm_rate_mismatch_mitigation`

* ionospheric correction writeout to CSLC

* removed unnecessary lines

* fixing codacy issue

* PEP8 issue

* updating runconfig template for CI

* revision on runconfig scheme and defaults

* PR comments addressed

* docstring revised

* revision on the description in corrections

* addressing comments in the 2nd round review

* PEP8

---------

Co-authored-by: Seongsu Jeong <seongsu.jeong@jpl.nasa.gov>
Co-authored-by: vbrancat <virginia.brancato@jpl.nasa.gov>
Co-authored-by: Liang Yu <liangjyu@gmail.com>
Co-authored-by: Heresh Fattahi <hersh.fattahi@gmail.com>
Co-authored-by: Zhang Yunjun <yunjunzgeo@gmail.com>
Co-authored-by: Scott Staniewicz <scott.j.staniewicz@jpl.nasa.gov>
* Correct inconsistencies inside CSLC-S1 metadata

* remove list of frequencies and diagnostic mode flag

* Correct spelling of ISCE to ISCE3

* Clarify some metadata description
* Add product version and processing type to the schema

* Allow the CSLC-S1 SAS to write product version and processing type

* Rename product_path to product_group

* Remove processing type

* Remove trailing space
* fix s1_geocode_metadata block params
* account for float only interpolator
* fix correction lut_to_hdf5 logic
Co-authored-by: Scott Staniewicz <scott.stanie@gmail.com>
opera-adt#113)

* Apply static troposphere model when weather model file is not provided

* whitespace removed

* Update src/compass/utils/lut.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* remove static troposphere correction LUT from the product

* docstring revised

* suggested change on workflow test code

* ionospere LUT writes into product

---------

Co-authored-by: Seongsu Jeong <seongsu.jeong@jpl.nasa.gov>
Co-authored-by: Liang Yu <liangjyu@gmail.com>
* initial commit for quality assurance

* browse image

* add log scale

* check if database file exists

* clip by percentage
resize to 2048 max dim
plot to gray scale with transparency

* add get_transform method

* use GDAL warp to output to WGS84

* cleaner NETCDF implementation

Co-authored-by: Scott Staniewicz <scott.stanie@gmail.com>

* get WGS84 extents for gdal.Warp

* add q_a params to yaml and runconfig

* untested integration of browse image into s1_geocode_slc

* browse image functions

* add stats compute and output to json

* add stats class

* add docstrings for stats
fix bugs

* codacy issues

* grow scope stats to include all of QA

* fix import error

* update orbit QA
QA items include description with value

* just in case weirdness in path

Co-authored-by: Scott Staniewicz <scott.stanie@gmail.com>

* expand hi/lo to high/low

---------

Co-authored-by: Scott Staniewicz <scott.stanie@gmail.com>
…t#114)

* Placeholder for geocoded radiometric calibration parameters

* geocoded calibration parameter feature as standalone function in `s1_geocode_metadata.py`

* Update src/compass/s1_geocode_metadata.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* Update src/compass/s1_geocode_metadata.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* Update src/compass/s1_geocode_metadata.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* Apply suggestions from code review by @LiangJYu

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* finalizing applying the code change suggestion

* removed unused imports

* geocode calibration LUTS only when it is available in burst object

* Moving all calibration info. into `metadata/calibration_information`; removed irrelevant calibration info in geogrid.

* `dn` added for geocoded LUT; note added for placeholder

* removed unused codes

* addressing codacy issue

* Update src/compass/s1_geocode_metadata.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

---------

Co-authored-by: Seongsu Jeong <seongsu.jeong@jpl.nasa.gov>
Co-authored-by: Liang Yu <liangjyu@gmail.com>
* fix clip and gamma correction bugs
* add standalone mode
* updating to defaults

Co-authored-by: Scott Staniewicz <scott.stanie@gmail.com>
* noise info moved to `metadata/noise_information`

* `geocode_noise_luts` implemented

* removed radargrid-based information in `noise_information`

* codacy issue

* reduce duplicate code

* fix on `lut_path` definition

* Apply suggestions from code review

Co-authored-by: Liang Yu <liangjyu@gmail.com>

---------

Co-authored-by: Seongsu Jeong <seongsu.jeong@jpl.nasa.gov>
Co-authored-by: Liang Yu <liangjyu@gmail.com>
…ra-adt#117)

* write x/y coords/spacings to topo.h5
* move burst ID grouping to helpers
* init geocoded dataset returns dataset
* output rename to static_layers_<burst_id>.h5
* static layer h5 structured like CSLC product
* write burst metadata to static layer HDF5
* specifile.txt updated

* specifile update after installing pysolid; manual addition of raider

* pyproj added into specifile

* raider installation from source code

* strategy change for RAiDER installation

* specfile updated

* specfile update after installing `pyproj`

* specfile update after installing `xarray`

* specfile update after installing `progressbar`

* removing jupyter notebook committed by mistake; update on specfile after installing `progressbar`

* specfile update after installing the remaining dependencies for RAiDER

* Dockerfile update

* hard-coding `s1-reader` version for gamma delivery

* Docker labe changed for gamma delivery

* specfile update after installing `pytest`

* specifile -> specfile

---------

Co-authored-by: Seongsu Jeong <seongsu.jeong@jpl.nasa.gov>
LiangJYu and others added 9 commits March 31, 2023 13:40
…opera-adt#124)

* validation modified to handle both CSLC and static layer HDF5s
* Add hashbang to enable scripting

Co-authored-by: Seongsu Jeong <sjeong.kr@gmail.com>
* write stats to HDF5
* add iono and SET correction stat computations
* include RAiDER tropo correction in stat computations
* add common function write to HDF5 and dict
* add QA to static layers HDF5
Co-authored-by: Seongsu Jeong <seongsu.jeong@jpl.nasa.gov>
Co-authored-by: Liang Yu <liangjyu@gmail.com>
Copy link
Contributor

@vbrancat vbrancat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @seongsujeong, I do not have major comments at this stage. We discussed the algorithm in our CSLC tag-up meeting and the implementation looks fine to me.

I have noticed that your branch contains commits from PRs which have been already merged. You might want to rebase this branch to avoid this https://www.google.com/search?q=git+merge+war&tbm=isch&ved=2ahUKEwjs9rjl8Iv-AhWiPkQIHfpEDSEQ2-cCegQIABAA&oq=git+merge+war&gs_lcp=CgNpbWcQAzoHCAAQigUQQzoFCAAQgAQ6BggAEAgQHjoHCAAQgAQQGFDlBVjCDWDvEGgAcAB4AIABOogBkwKSAQE1mAEAoAEBqgELZ3dzLXdpei1pbWfAAQE&sclient=img&ei=sdApZKzaNaL9kPIP-om1iAI&bih=800&biw=1454&rlz=1C5GCEM_en#imgrc=PImq13_QtYFdDM while merging. An alternative is to hand-pick your commits and submit a PR using the main branch.

@@ -265,3 +266,122 @@ def get_unit_vector4component_of_interest(los_inc_angle, los_az_angle, comp='enu
]

return unit_vec

def enu2rgaz(radargrid_ref, orbit, ellipsoid,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to separate this function in a enu2los and en2az? Just in case, we want to use them separately in the code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conversion in this PR is based on geo2rdr, meaning that it will calculate range and azimuth displacements simultaneously. I'm not sure how much useful separating them would be.
In case we want to use only one of the components, I think we can suppress the unnecessary return values. For example,

rg_arr, _ = enu2rgaz(radargrid_ref, orbit, ellipsoid, ...)
_, az_arr = enu2rgaz(radargrid_ref, orbit, ellipsoid, ...)

src/compass/utils/geometry_utils.py Show resolved Hide resolved
-------
rg_arr: np.ndarray
Displacement in slant range direction in meters.
az_arr: np.ndarray
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm, what do you think of separating the functionalities? 1) Have a conversion from en2az where the azimuth component is in meters 2) convert azimuth component from meters to seconds.

I can see both these functionalities be reused in several parts of the code

Copy link
Contributor Author

@seongsujeong seongsujeong Apr 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same response as above.

isce3.core.LUT2d(),
radargrid_ref.wavelength,
radargrid_ref.lookside,
threshold=1.0e-10, maxiter=50, delta_range=10.0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a comment on how we got the numbers for the threshold and delta_range?

src/compass/utils/geometry_utils.py Show resolved Hide resolved
src/compass/utils/geometry_utils.py Show resolved Hide resolved
lat: np.ndarray
Latitude of the points to calculate ENU vectors
unit: str
Unit of the `lon` and `lat`. Default is `degree`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's document what are the other plausible units that the code support

Comment on lines +378 to +379
np.sin(lon_rad) * np.cos(lat_rad),
np.sin(lat_rad)])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this. I've adjusted the indentation spaces.

@seongsujeong seongsujeong deleted the set_az_conversion branch April 4, 2023 21:41
seongsujeong pushed a commit to seongsujeong/COMPASS that referenced this pull request Apr 4, 2023
@seongsujeong seongsujeong mentioned this pull request Apr 4, 2023
seongsujeong added a commit that referenced this pull request May 23, 2023
* revised rg / az conversion

* three different versions of `solid_earth_tides` for record

* clean up version + original `solid_earth_tides`

* Update src/compass/utils/lut.py

Co-authored-by: Liang Yu <liangjyu@gmail.com>

* unused function removed

* `enu2rgaz` and `get_enu_vector_ecef` moved to `geometry_utils.py`

* updates

* Applying suggestions in PR #105

* brings geo2rdr parameters from cfg; `tide_az` added

* codacy issue

* addressing comments in the PR

* azimuth SET added into `corrections_to_h5group`; correction layer order adjusted

* Addressing comments provided by @scottstanie

* Update src/compass/utils/lut.py

Co-authored-by: Scott Staniewicz <scott.stanie@gmail.com>

* pass geo2rdr parameter into `compute_geocoding_correction_luts`

---------

Co-authored-by: Seongsu Jeong <seongsu.jeong@jpl.nasa.gov>
Co-authored-by: Liang Yu <liangjyu@gmail.com>
Co-authored-by: Scott Staniewicz <scott.stanie@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants