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

Update to python 3.10+ #162

Open
wants to merge 22 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: ["3.9"]
python-version: ["3.9", "3.10", "3.11"]

steps:
- uses: actions/checkout@v3
Expand Down
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ and uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
* Records parameters in the product including the CLI command to regenerate said product
* If parameters are not standard uses prefix `S1-GUNW_CUSTOM-...`
* Pydantic dependency for parameter accounting
* Support for python 3.10 and 3.11 (Note: 3.12 cannot be supported due to pysolid issue: https://github.com/insarlab/PySolid/issues/78)

### Changed
* The CLI now *requires* `frame_id` (use `frame_id = -1` for old API and what is now considered a "non"-standard product)
* Water mask now uses `tile-mate` to download and merge ESA World cover tiles on
* All water masks applied to processing/packaging use ESA 10 meter world cover: ionosphere processing, browse imagery, and global attributes associate with mean coherence
* Water mask now uses `tile-mate>=0.0.8` to download and merge water mask tiles (Pekel Occurence data >= 95 is the default)
* All water masks applied to processing/packaging use Pekel Occurence (>= 95 percent occurence): ionosphere processing, browse imagery, and global attributes associate with mean coherence
* Some function names associated to writing global attributes in the netcdf file were renamed to be more descriptive e.g. `record_stats` became `record_stats_as_global_attrs`

## [0.3.0]
Expand Down
24 changes: 20 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,21 @@ We note all the input datasets are publicly available using a NASA Earthdata acc
3. Create a new environment and install requirements using `conda env update --file environment.yml` (or use [`mamba`](https://github.com/mamba-org/mamba) to speed install up)
4. Install the package from cloned repo using `python -m pip install -e .`

### For Mac M1 Silicon Users

`ISCE2` requires Intel `x86_64` complied, conda-forge packages. Please follow the directions [here](https://conda-forge.org/docs/user/tipsandtricks.html#installing-apple-intel-packages-on-apple-silicon) i.e.

```
CONDA_SUBDIR=osx-64 conda create -n topsapp_env python
conda activate topsapp_env
conda config --env --set subdir osx-64
```
Then check
```
python -c "import platform;print(platform.machine())" # Should print "x86_64"
echo "CONDA_SUBDIR: $CONDA_SUBDIR" # Should print "CONDA_SUBDIR: osx-64"
```

## Additional setup

1. Ensure that your `~/.netrc` file has:
Expand Down Expand Up @@ -158,7 +173,6 @@ or as a json:
## Expedient Docker Test for GUNW Generation

Create a new directory (for all the intermediate files) and navigate to it.

```
docker run -ti -v $PWD:/home/ops/topsapp_data topsapp_img \
--reference-scenes S1A_IW_SLC__1SDV_20220212T222803_20220212T222830_041886_04FCA3_2B3E \
Expand Down Expand Up @@ -209,8 +223,8 @@ EARTHDATA_PASSWORD=...
The main entrypoint is in the `__main__.py` file [here](https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsApp/blob/dev/isce2_topsapp/__main__.py).
The whole standard `++slc` workflow that generates the `ARIA-S1-GUNW` product is summarized in this file.
The workflow takes 1.5 - 3 hours to complete.
For developing a new feature, we need to modify a very small parts of this long running workflow e.g. the packaging of ISCE2 outputs into a netcdf file, staging/preparation of auxiliary data, etc.
Thus, for development it is not necessary to rerun the workflow each time to test a new feature, but utilize the intermediate ISCE data files and fix relevant parts of the code.
Likely, when developing a small new feature, we need to modify only of this long running workflow e.g. the packaging of ISCE2 outputs into a netcdf file, staging/preparation of auxiliary data, etc.
Thus, for development, it is recommended to not rerun the workflow each time, but utilize the intermediate ISCE data files and the metadata stored as json by this plugin.
We have some sample [notebooks](https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsapp-Debugging-NBs) to load the relevant metadata to make this "jumping" into the code slightly easier.
We note that `ISCE2` generates *a lot* of interemdiate files.
For our workflow, this can be between 100-150 GBs of disk required.
Expand All @@ -225,6 +239,7 @@ However, the plugin takes about 1.5 to 3 hours (depending on the number of corre
Therefore, these integration are *not* sufficient to permit a new release (see more instructions below).
Until we have a complete end-to-end test of the workflow (via Hyp3), any new feature cannot be integrated into official production (i.e. the `main` branch).
As a first step, it is imperative to share the output of a new feature (i.e. the GUNW file and the command to generate it).
Here is a notebook that demonstrates how to compare GUNWs that will be very helpful: https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsapp-Debugging-NBs/blob/dev/2_Compare_GUNWs.ipynb


## Hyp3 and Cloud Submission
Expand Down Expand Up @@ -261,9 +276,10 @@ Here are more detailed [notes](https://github.com/ACCESS-Cloud-Based-InSAR/CICD-

### Additional (manual) checks

Even though there are some integration tests and unit tests in our test suites, this CPU intensive workflow cannot be tested end-to-end using github actions (there is simply not enough memory and CPU on a github runner).
Even though there are some integration tests and unit tests in our test suites, this CPU intensive workflow cannot be tested end-to-end using github actions (there is simply not enough memory and CPU for these workflows).
Therefore, even if all the tests pass, there is still a nontrivial chance a GUNW is not successfully generated (e.g. the `topo` step of topsapp fizzles out because `numpy` API was not successfully tracked in the latest ISCE2 release).
We request a sample GUNW be shared in a PR.
Ideally, a comparison of the GUNW created with a new branch and an existing one (as done in this [notebook](https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsapp-Debugging-NBs/blob/dev/2_Compare_GUNWs.ipynb)) is ideal.
Even if we go through careful accounting, once a PR is merged into `dev`, we will use `hyp3` to further inspect the new plugin e.g. through this [notebook](https://github.com/ACCESS-Cloud-Based-InSAR/s1_frame_enumerator/blob/dev/notebooks/Submitting_to_Hyp3.ipynb) using a few sites.
It is important to use `INSAR_ISCE_TEST` job to ensure the features from the `dev` branch are used.
Only after these **manual** checks, will we continue with a release of the plugin.
Expand Down
6 changes: 3 additions & 3 deletions environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: topsapp_env
channels:
- conda-forge
dependencies:
- python>=3.9,<3.10
- python>=3.9,<3.12
- pip
- affine
- asf_search>=5.0.0
Expand All @@ -17,7 +17,7 @@ dependencies:
- geopandas
- hyp3lib>=3,<4
- ipykernel
- isce2==2.6.1
- isce2==2.6.3
- jinja2
- joblib
- jsonschema==3.2.0
Expand All @@ -44,4 +44,4 @@ dependencies:
- tqdm
- dem_stitcher>=2.5.5
- aiohttp # only needed for manifest and swath download
- tile_mate>=0.0.7
- tile_mate>=0.0.8
2 changes: 1 addition & 1 deletion isce2_topsapp/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def localize_data(
# For ionospheric correction computation
if water_mask_flag:
out_water_mask = download_water_mask(
out_slc["extent"], water_mask_name="esa_world_cover_2021_10m")
out_slc["extent"], water_mask_name="pekel_water_occurrence_2021")

out_aux_cal = download_aux_cal()

Expand Down
18 changes: 17 additions & 1 deletion isce2_topsapp/localize_mask.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@


def download_water_mask(
extent: list, water_mask_name: str = "esa_world_cover_2021_10m", buffer: float = 0.1
extent: list, water_mask_name: str = "pekel_water_occurrence_2021", buffer: float = 0.1
) -> dict:
output_dir = Path(".").absolute()

Expand Down Expand Up @@ -39,6 +39,22 @@ def download_water_mask(

mask_filename = fix_image_xml(mask_filename)

elif water_mask_name == 'pekel_water_occurrence_2021':
X, p = get_raster_from_tiles(extent_buffered, tile_shortname='pekel_water_occ_2021')
mask = (X >= 95).astype(np.uint8)
mask[mask.astype(bool)] = 255
mask_filename = 'water_mask_derived_from_pekel_water_occurrence_2021_with_at_least_95_perc_water.geo'

# Remove esa nodata, change gdal driver to ISCE, and generate VRT as in localize DEM
p_isce = p.copy()
p_isce['nodata'] = None
p_isce['driver'] = 'ISCE'

with rasterio.open(mask_filename, 'w', **p_isce) as ds:
ds.write(mask)

mask_filename = fix_image_xml(mask_filename)

elif water_mask_name == "SWBD":
# Download SRTM-SWDB water mask
# Water mask dataset extent
Expand Down
4 changes: 2 additions & 2 deletions isce2_topsapp/packaging_utils/nc_packaging.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def write_dataset(fid,data,properties_data):
else:

if isinstance(data,str):
dset = fid.createVariable(properties_data.name, str, ('matchup',), zlib=True)
dset = fid.createVariable(properties_data.name, str, ('matchup',))
dset[0]=data
elif isinstance(data,np.ndarray):
# make sure the _fillvalue is formatted the same as the data_type
Expand All @@ -156,7 +156,7 @@ def write_dataset(fid,data,properties_data):
dset[:] = data
elif isinstance(data, collections.abc.Iterable):
if isinstance(data[0],str):
dset = fid.createVariable(properties_data.name, str, ('matchup',), zlib=True)
dset = fid.createVariable(properties_data.name, str, ('matchup',))
count = 0
for data_line in data:
dset[count]=data_line
Expand Down
6 changes: 3 additions & 3 deletions isce2_topsapp/water_mask.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def get_water_mask_raster_for_browse_image(profile: dict) -> np.ndarray:
"""
extent = array_bounds(profile["height"], profile["width"], profile["transform"])

X_esa, p_esa = get_raster_from_tiles(extent, tile_shortname="esa_world_cover_2021")
X_esa_r, _ = reproject_arr_to_match_profile(X_esa, p_esa, profile, resampling='nearest')
mask = (X_esa_r == 80).astype(bool)
X_occ, p_occ = get_raster_from_tiles(extent, tile_shortname="pekel_water_occ_2021")
X_occ_r, _ = reproject_arr_to_match_profile(X_occ, p_occ, profile, resampling='bilinear')
mask = (X_occ_r >= 95).astype(bool)
mask = mask[0, ...]
return mask
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
'License :: OSI Approved :: Apache Software License',
'Natural Language :: English',
'Programming Language :: Python :: 3',
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: 3.9',
'Programming Language :: Python :: 3.10',
'Programming Language :: Python :: 3.11',
],

python_requires='>=3.8',
Expand Down
2 changes: 1 addition & 1 deletion tests/test_packaging.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def test_mean_of_geocoded_isce_outputs():
"mean_filtered_coherence_without_water_mask": 0.4395995,
"mean_filtered_coherence_with_water_mask": 0.4649538,
"mean_unfiltered_coherence_without_water_mask": 0.33125195,
"mean_unfiltered_coherence_with_water_mask": 0.33531728,
"mean_unfiltered_coherence_with_water_mask": 0.3347433,
"mean_incidence_angle": 38.845047,
"mean_azimuth_angle": -169.84756469726562,
}
Expand Down
Loading