ACCESS-Cloud-Based-InSAR / DockerizedTopsApp

Apache License 2.0
21 stars 2 forks source link

Move isce2 shenanigans to __init__ so we only have to do it once #175

Closed jhkennedy closed 6 months ago

jhkennedy commented 6 months ago

Handling ISCE2 shenanigans in the isce2_topsapp.__init__ before we import other parts of isce2_topsapp ensures we only have to do it once, no matter where we are in the package. Specifically, this:

I additionally removed some unused imports I noticed.

jhkennedy commented 6 months ago

@cmarshak definitely run a test job or few before releasing this :smile:

cmarshak commented 6 months ago

@jhkennedy - certainly will test before next release :)

Note that the orbit is failing in the tests - will rerun - but just wanted you to see.

42m 16s
  /home/runner/micromamba/envs/topsapp_env/lib/python3.9/site-packages/dem_stitcher/rio_window.py:99: RuntimeWarning: Requesting extent beyond raster bounds of [-70.0, 40.0, -60.0, 50.0]. Shrinking bounds in raster crs to (-70.0, 47.460101016, -67.38343517999999, 48.669267199000004).
    warn(

tests/test_packaging.py::test_mean_of_geocoded_isce_outputs
  /home/runner/micromamba/envs/topsapp_env/lib/python3.9/site-packages/rasterio/__init__.py:[304](https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsApp/actions/runs/8073767996/job/22057989747?pr=175#step:5:305): NotGeoreferencedWarning: Dataset has no geotransform, gcps, or rpcs. The identity matrix will be returned.
    dataset = DatasetReader(path, driver=driver, sharing=sharing, **kwargs)

tests/test_set.py::test_set_workflow
  <frozen importlib._bootstrap>:228: RuntimeWarning: numpy.ndarray size changed, may indicate binary incompatibility. Expected 16 from C header, got 96 from PyObject

tests/test_set.py::test_set_workflow
tests/test_set.py::test_magnitude_of_set_with_variable_timing[reference]
tests/test_set.py::test_magnitude_of_set_with_variable_timing[reference]
tests/test_set.py::test_magnitude_of_set_with_variable_timing[secondary]
tests/test_set.py::test_magnitude_of_set_with_variable_timing[secondary]
  /home/runner/micromamba/envs/topsapp_env/lib/python3.9/site-packages/isce2_topsapp/solid_earth_tides.py:[310](https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsApp/actions/runs/8073767996/job/22057989747?pr=175#step:5:311): FutureWarning: 'S' is deprecated and will be removed in a future version, please use 's' instead.
    dt_sec_floor = dt.floor('S')

tests/test_set.py::test_set_workflow
tests/test_set.py::test_magnitude_of_set_with_variable_timing[reference]
tests/test_set.py::test_magnitude_of_set_with_variable_timing[reference]
tests/test_set.py::test_magnitude_of_set_with_variable_timing[secondary]
tests/test_set.py::test_magnitude_of_set_with_variable_timing[secondary]
  /home/runner/micromamba/envs/topsapp_env/lib/python3.9/site-packages/isce2_topsapp/solid_earth_tides.py:[311](https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsApp/actions/runs/8073767996/job/22057989747?pr=175#step:5:312): FutureWarning: 'S' is deprecated and will be removed in a future version, please use 's' instead.
    dt_sec_ceil = dt.ceil('S')

tests/test_set.py: 80 warnings
  /home/runner/micromamba/envs/topsapp_env/lib/python3.9/site-packages/rioxarray/_io.py:488: DeprecationWarning: string or file could not be read to its end due to unmatched data; this will raise a ValueError in the future.
    new_val = np.fromstring(value.strip("{}"), dtype="float", sep=",")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
FAILED tests/test_notebooks.py::test_notebooks[solid_earth_tides.ipynb] - papermill.exceptions.PapermillExecutionError: 
---------------------------------------------------------------------------
Exception encountered at "In [7]":
---------------------------------------------------------------------------
OrbitDownloadError                        Traceback (most recent call last)
Cell In[7], line 2
      1 esa_credentials = (os.environ['ESA_USERNAME'], os.environ['ESA_PASSWORD'])
----> 2 orb_file, _ = get_orb.downloadSentinelOrbitFile(slc_id, esa_credentials=esa_credentials, providers=('ASF', 'ESA'))
      3 orb_file

File ~/micromamba/envs/topsapp_env/lib/python3.9/site-packages/hyp3lib/get_orb.py:189, in downloadSentinelOrbitFile(granule, directory, providers, orbit_types, esa_credentials)
    184             logging.warning(
    185                 f'Error encountered fetching {orbit_type} orbit file from {provider}; looking for another',
    186             )
    187             continue
--> 189 raise OrbitDownloadError(f'Unable to find a valid orbit file from providers: {providers}')
jhkennedy commented 6 months ago

@cmarshak thanks! Yes, orbits are definitely unstable right now.

cmarshak commented 6 months ago

Just submitted 10 jobs - works great (8/10 success and the 2 failures were clearly terminations).

Unfortunately, there are times when I import ISCE2 as a library and it appears to reset the entire logging.

I think this is an overall improvement and we can identify other places where we can adapt this approach to further improve the logging.

Here are some likely culprits:

https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsApp/blob/dev/isce2_topsapp/solid_earth_tides.py#L8-L22 https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsApp/blob/dev/isce2_topsapp/iono_proc.py#L5-L22

cmarshak commented 6 months ago

Here is a sample log from one of the terminations: https://hyp3-a19-jpl-contentbucket-1wfnatpznlg8b.s3.us-west-2.amazonaws.com/05881b0b-f2ac-4208-ae83-afa782da6b43/05881b0b-f2ac-4208-ae83-afa782da6b43.log

cmarshak commented 6 months ago

Actually - it's probably these pieces of code:

https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsApp/blob/dev/isce2_topsapp/packaging_utils/nc_packaging.py#L19-L21

https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsApp/blob/dev/isce2_topsapp/packaging_utils/makeGeocube.py#L22-L24

Might be good to redirect these to a log file or something else. For another time :)