-
Notifications
You must be signed in to change notification settings - Fork 241
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
save_hdfeos5.py: use mintpy.subset for naming if attributes missing #938
base: main
Are you sure you want to change the base?
Conversation
geom_meta = readfile.read_attribute(geom_file) # FA 12/22: for TSX meta does not have relative_orbit attribute | ||
unavco_meta['relative_orbit'] = int(geom_meta['unavco.relative_orbit']) | ||
except ValueError: | ||
print('Missing required attribute: relative_orbit') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the motivation of this change here: time-series file is supposed to have more metadata than geometry files, always. Isn't that the case for your dataset?
To ensure we are on the same page on the details: the unavco.relative_orbit
should had been translated into relative_orbit
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be that this is required for MiaplPy and that load_slc_geometry.py
of MiaplPy does not use this MintPy function. relative_orbit
in slcStack.h5
is currently none as you can see below. Maybe we should make a note how how this could be improved.
MiamiTsxSMD14/miaplpy/inputs[1014] info.py slcStack.h5
******************** Basic File Info ************************
file name: /scratch/05861/tg851601/MiamiTsxSMD14/miaplpy/inputs/slcStack.h5
file type: timeseries
coordinates : RADAR
******************** Date Stat Info *************************
Start Date: 20170923
End Date: 20210618
Number of dates : 100
STD of datetimes : 1.08 years
******************** HDF5 File Structure ********************
Attributes in / level:
ALOOKS 1
ANTENNA_SIDE -1
AZIMUTH_PIXEL_SIZE 2.1495932845300736
BANDS 1
CENTER_LINE_UTC 40691.0
DATA_TYPE complex64
DATE 20200529
EARTH_RADIUS 6348107.616403343
FILE_LENGTH 7046
FILE_PATH /scratch/05861/tg851601/MiamiTsxSMD14/merged/SLC/20200529/20200529.slc
FILE_TYPE timeseries
HEADING -168.4374330199127
HEIGHT 511035.8120019732
INTERLEAVE BIP
LAT_REF1 25.921521074257434
LAT_REF2 25.94302544748516
LAT_REF3 25.797651852239053
LAT_REF4 25.819170744512242
LENGTH 7046
LON_REF1 -80.02806403183072
LON_REF2 -80.16693071534473
LON_REF3 -80.05174311730326
LON_REF4 -80.19047876057562
NCORRLOOKS 0.8178098213569981
PLATFORM Tsx
POLARIZATION HH
PRF 3577.0218098958335
PROCESSOR isce
PROJECT_NAME MiamiTsxSMD14
P_BASELINE_BOTTOM_HDR -460.76757280546883
P_BASELINE_TOP_HDR -460.0380850366407
RANGE_PIXEL_SIZE 1.3641053359701238
RLOOKS 1
STARTING_RANGE 681478.0615665995
SUBSET_XMAX 7168
SUBSET_XMIN 0
SUBSET_YMAX 7046
SUBSET_YMIN 0
UNIT i
WAVELENGTH 0.03106658115133631
WIDTH 7168
XMAX 7167
YMAX 7045
access_mode read
altitude 511035.8120019732
azimuthPixelSize 2.1495932845300736
azimuthResolution 2.392
beam_mode SM
byte_order l
data_type complex64
description ['Single look complex image.']
earthRadius 6348107.616403343
extra_file_name /scratch/05861/tg851601/MiamiTsxSMD14/merged/SLC/20170923/20170923.slc.vrt
family slcimage
file_name /scratch/05861/tg851601/MiamiTsxSMD14/merged/SLC/20170923/20170923.slc
image_type slc
isce_version Release: 2.6.1, svn-, 20220811. Current: svn-.
length 7046
name slcimage_name
number_bands 1
orbitNumber 56649
polarization HH
prf 3577.0218098958335
radarWavelength 0.03106658115133631
rangePixelSize 1.3641053359701238
rangeResolution 1.49896229
relative_orbit None
satelliteSpeed 7689.142061169693
scheme BIP
startUTC 2017-09-01 11:18:06.651931
startingRange 681478.0615665995
stopUTC 2017-09-01 11:18:17.196737
trackNumber None
width 7168
xmax 7168
xmin 0
HDF5 dataset "/bperp ": shape=(100,) , dtype=float32 , compression=None
HDF5 dataset "/date ": shape=(100,) , dtype=|S8 , compression=None
HDF5 dataset "/slc ": shape=(100, 7046, 7168) , dtype=complex64 , compression=None
MODIFICATION_TIME 1672180816.6942618
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this change because it makes the code messy. I suggest grabbing the valid metadata, such as relative_orbit
, from the geometry file here, if that has more metadata than the time-series file.
IMO, both files should have these types of metadata consistent in the miaplpy stage.
lat0 = lat1 + float(metadata['Y_STEP']) * int(metadata['LENGTH']) | ||
lon1 = lon0 + float(metadata['X_STEP']) * int(metadata['WIDTH']) | ||
elif 'mintpy.subset.lalo' in template.keys(): | ||
# for MiaplPy it would be preferrd to use miaplpy.subset.lalo but that is not available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Then let's modify the save_hdfeos5.py -t
option to 1) be able to take multiple template files, 2) read the miaplpy template file and pass it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean adding -t
for use outside smallbaselineApp
with MiaplPy? That would make sense. But I think this is not only a MiaplPy issue. I believe I saw it occasionally for MintPy as well.
My preference would be to accept as is but make a note in the code that I will implement the -t option when I get a chance. After that we can switch back and we will see whether it really works.
The big question is what to do with Sara's geolocation correction and how to save the corrected coordinates in the S1 file. It would be good to implement this into MintPy but I don't see that Sara will have time for that any time soon. Unless you want to do it I probably will write a minsar script that would use save_hdfeos5.py -t
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant to modify save_hdfeos5.py
to read both template files from mintpy and miaplpy, so that we could use miaplpy.subset.lalo
as you wrote in the code comment.
My preference would be to accept as is but make a note in the code that I will implement the -t option when I get a chance. After that we can switch back and we will see whether it really works.
Sounds fine to me.
if suffix: | ||
outName = fbase.removesuffix('_' + suffix) + SUB + '_' + suffix + fext | ||
else: | ||
outName = fbase + SUB + fext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the suffix
is used here, I don't see the necessity of this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you referring to removesuffix? Well, it was needed. Otherwise I would not have added it. fbase seems to have occasionally an _
.
You could make a mote to the code that this might be unnecessary and I can revisit next time I deal with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide enough detailed examples, so that I could test this change. Otherwise, we can not accept the PR here.
Thanks @falkamelung. The general motivation sounds reasonable to me. I have a few comments on the implementation details. |
This reverts commit 2da8cae.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what I have to say in this box. I did these changes weeks ago and in the next weeks it may be difficult to find the time to thoroghly test additional modifications.
I don't think anybody is using HDF5EOS anyway???
if suffix: | ||
outName = fbase.removesuffix('_' + suffix) + SUB + '_' + suffix + fext | ||
else: | ||
outName = fbase + SUB + fext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you referring to removesuffix? Well, it was needed. Otherwise I would not have added it. fbase seems to have occasionally an _
.
You could make a mote to the code that this might be unnecessary and I can revisit next time I deal with this.
Description of proposed changes
save_hdf5eos.py
occasionally fails for subsets when attributes about the subset are missing because the subset information is used for the filename creation. With this PR, instead of failing, it uses themintpy.subset
information to generate the file name.This is needed for MiaplPy because it uses SLCs which miss some attributes. I believe I have seen this error also for running mintpy only, but I forgot details.
Reminders