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

Sb/refpixel lat lon #272

Merged
merged 25 commits into from
Jun 24, 2020
Merged

Sb/refpixel lat lon #272

merged 25 commits into from
Jun 24, 2020

Conversation

basaks
Copy link
Contributor

@basaks basaks commented Jun 15, 2020

Adds lat/lon refpixel and tests around it

@basaks basaks requested review from mcgarth and tfuhrmann June 15, 2020 18:21
@codecov-commenter
Copy link

codecov-commenter commented Jun 15, 2020

Codecov Report

Merging #272 into develop will increase coverage by 0.36%.
The diff coverage is 96.49%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #272      +/-   ##
===========================================
+ Coverage    92.78%   93.14%   +0.36%     
===========================================
  Files           25       26       +1     
  Lines         3089     3195     +106     
  Branches       494      495       +1     
===========================================
+ Hits          2866     2976     +110     
+ Misses         138      137       -1     
+ Partials        85       82       -3     
Impacted Files Coverage Δ
pyrate/core/refpixel.py 91.81% <93.10%> (-1.05%) ⬇️
pyrate/core/ifgconstants.py 100.00% <100.00%> (ø)
pyrate/core/prepifg_helper.py 95.17% <100.00%> (+0.03%) ⬆️
pyrate/core/shared.py 95.24% <100.00%> (+2.20%) ⬆️
pyrate/prepifg.py 97.61% <100.00%> (-1.57%) ⬇️
pyrate/process.py 91.78% <100.00%> (+0.31%) ⬆️
pyrate/constants.py 100.00% <0.00%> (ø)

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 20507ba...3f25d50. Read the comment docs.

Base automatically changed from sb/file-types-handling to develop June 16, 2020 11:27
Copy link
Contributor

@tfuhrmann tfuhrmann left a comment

Choose a reason for hiding this comment

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

I've checked this PR and can confirm that the functionality works in general (i.e. a given lat/lon coordinate is used for reference area calculation). I've checked two different reference area locations for a real-world data stack and the resulting stack_rate outputs were different (which is as expected).
I realise a difference by one pixel in x and y for both tested reference areas when checking the transformation of lat/lon input using gdallocationinfo (see below). Not sure if this is a rounding issue or a small bug?

PyRate config file input (ref area 1):
refx: 150.87
refy: -34.00
PyRate command line output: Converted reference pixel coordinate (x, y): (564, 228)
gdallocationinfo -geoloc stack_rate.tif 150.87 -34.00
Report:
Location: (563P,227L)
Band 1:
Value: -1.20118641853333
[txf547@gadi-login-03 out]$ gdallocationinfo stack_rate.tif 564 228 Report:
Location: (564P,228L)
Band 1:
Value: -0.00996732711791992

PyRate config file input (ref area 2):
refx: 150.56
refy: -34.19
PyRate command line output: Converted reference pixel coordinate (x, y): (117, 501)
[txf547@gadi-login-03 out]$ gdallocationinfo -geoloc stack_rate.tif 150.56 -34.19
Report:
Location: (116P,500L)
Band 1:
Value: -12.3063049316406
[txf547@gadi-login-03 out]$ gdallocationinfo stack_rate.tif 117 501
Report:
Location: (117P,501L)
Band 1:
Value: -21.5510654449463

@tfuhrmann
Copy link
Contributor

I've re-run my real data test using the latest commit (rounding). The lat/lon to pixel coordinate conversion is now delivering exactly the same output as gdallocationinfo for the first reference area, but still off by one pixel for the second reference area (see below command line output). I think the small difference of one pixel for some locations is accectable.

PyRate command line output:
ref area 1: Converted reference pixel coordinate (x, y): (563, 227)
ref area 2: Converted reference pixel coordinate (x, y): (117, 501)

Copy link
Contributor

@mcgarth mcgarth left a comment

Choose a reason for hiding this comment

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

The functionality works as expected. Once the tests pass then this can be merged

@basaks basaks force-pushed the sb/refpixel-lat-lon branch from 007f858 to 3f25d50 Compare June 23, 2020 20:18
@basaks basaks force-pushed the sb/refpixel-lat-lon branch from 43edb03 to 226cd59 Compare June 24, 2020 00:06
@mcgarth mcgarth merged commit 79b5d29 into develop Jun 24, 2020
@mcgarth mcgarth deleted the sb/refpixel-lat-lon branch June 24, 2020 00:53
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

Successfully merging this pull request may close these issues.

5 participants