-
Notifications
You must be signed in to change notification settings - Fork 260
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
Load ionosphere time series from topsStack processor #1019
base: main
Are you sure you want to change the base?
Conversation
Thank you @yuankailiu for the big PR. I want to let you know that it may take some time for me to find time and review it. In the meanwhile, I will cut for a new release (version 1.5.2) before merging this PR. |
c22d180
to
427ef95
Compare
1. new template keywords for the ion timeseries: + mintpy.load.ionFile: ../ion_dates/*.ion #[path pattern of ionosphere timeseries files] + mintpy.load.ionBurstRampFile = ../ion_burst_ramp_merged_dates/*.float #[path pattern of ionosphere burst ramp timeseries files] 2. load_data: more flexible loading datasets + allow to specify which dataset to load with -l option. choice from {ifg/offset, geom, ion} 3. stackDict: for ion timeseries files from topsStack + class timeseriesAcqDict(): read(): data_unit & phase2range
427ef95
to
dd6dbe9
Compare
@sourcery-ai review and summary |
Reviewer's Guide by SourceryThis pull request introduces significant changes to handle ionosphere correction in MintPy. The main focus is on loading ionosphere time series products directly from ISCE2/topStack processor, rather than loading ionospheric interferograms and performing SBAS inversion. The changes include updates to data loading, new template keywords, and modifications to handle the new ionosphere data format. Sequence diagram for loading ionosphere time seriessequenceDiagram
participant User
participant MintPy
participant ISCE2
User->>MintPy: load_data.py -t smallbaselineApp.cfg -l ion
MintPy->>ISCE2: Request ionosphere time series files
ISCE2-->>MintPy: Return .ion and .float files
MintPy->>MintPy: Process and load ionosphere time series
MintPy-->>User: Save ion.h5 and ionBurstRamp.h5
Class diagram for new timeseriesDict and timeseriesAcqDict classesclassDiagram
class timeseriesDict {
-name: str
-datesDict: dict
-dsName0: str
+get_size(box, xstep, ystep, geom_obj): tuple
+get_date_list(): list
+get_dataset_list(): list
+get_metadata(): dict
+get_dataset_data_type(dsName): str
+write2hdf5(outputFile, access_mode, box, xstep, ystep, mli_method, compression, extra_metadata, geom_obj): str
}
class timeseriesAcqDict {
-name: str
-datasetDict: dict
-metadata: dict
+read(family, box, xstep, ystep, mli_method, resize2shape): tuple
+get_size(family): tuple
+get_perp_baseline(family): float
+get_metadata(family): dict
+get_dataset_list(): list
}
timeseriesDict --> timeseriesAcqDict : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @yuankailiu - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding unit tests for the new ionospheric correction functionality to ensure robustness and catch potential regressions.
- Review error handling in new functions, especially for file I/O operations, to improve reliability and provide clearer error messages to users.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@@ -411,6 +412,397 @@ def get_metadata(self, family=IFGRAM_DSET_NAMES[0]): | |||
return self.metadata | |||
|
|||
|
|||
######################################################################################## | |||
class timeseriesDict: |
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.
suggestion: Consider refactoring timeseriesDict to reduce code duplication
The new timeseriesDict class seems to duplicate functionality from ifgramStackDict. Consider refactoring to create a common base class or use composition to reduce code duplication and improve maintainability.
class TimeSeriesDict(IfgramStackDict):
"""
TimeSeriesDict object for a set of InSAR acquisitions from the same platform and track.
Inherits from IfgramStackDict to reduce code duplication.
"""
msg = 'WARNING: NOT all types of dataset have the same number of files.' | ||
msg += ' -> skip interferograms with missing files and continue.' | ||
print(msg) | ||
raise Exception(msg) |
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.
issue: Reconsider exception handling in read_inps_dict2timeseries_dict_object
Raising an exception with a warning message is an unusual pattern. Consider using a logging framework to log the warning and handle the error condition more gracefully, possibly by returning None or a partial result.
Outline
This PR is meant to better handle the ionosphere correction in MintPy. Before this PR, the ionosphere data was loaded into MintPy as ionospheric interferograms. We then do an SBAS inversion to get an ionospheric time series to do the correction (history).
As MintPy users, for simplicity, now, we want to load the ionosphere time series products from ISCE2/topStack processor directly. Once this #600 PR on ISCE is merged into topsStack code, the ionosphere final products contain the following files storing under the directory of topsStack processor,
$STACK_DIR
:$STACK_DIR/ion_dates/*.ion
: the smooth ionospheric time series for each epoch$STACK_DIR/ion_burst_ramp_merged_dates/*.float
: the burst ramp time series for each epochOnce the mintpy .cfg template file is filled with the above two paths, we can load those ionospheric time series directly into two separate time-series files
ion.h5
andionBurstRamp.h5
, respectively during theload_data.py
stage.The loaded ionosphere files
ion.h5
andionBurstRamp.h5
will be underinputs/
. Later, we can apply the correction by the following:Commits
Based on above, I also update the
defaults/auto_path.py
,defaults/smallbaselineApp.cfg
,objects/stack.py
.load_data.py
,cli/load_data.py
)load_data.py -l
option. choice from {ifg
,geom
,ion
}prep_isce.py
andstackDict.py
: read ion time-series files from topsStack filesreadfile.py
to read these isce .float and .ion filesDescription of proposed changes
Reminders
Summary by Sourcery
Add functionality to load ionosphere time series directly from the ISCE2/topStack processor into MintPy, streamlining the process of handling ionospheric corrections. Introduce new configuration options and enhance data loading flexibility.
New Features:
Enhancements:
load_data.py
script to allow selective loading of datasets, including ionosphere time series, geometry, and interferograms.